Skip to content
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

refactor publish workflow to use security, cleanup configuration #59

Merged
merged 17 commits into from Jul 14, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Expand Up @@ -3,11 +3,11 @@ language: php
php:
- 5.3
- 5.4
- 5.5

env:
# - SYMFONY_VERSION=2.1.*
- SYMFONY_VERSION=2.2.*
# - SYMFONY_VERSION=2.3.*
- SYMFONY_VERSION=2.3.*
# - SYMFONY_VERSION=dev-master

before_script:
Expand Down
Expand Up @@ -6,11 +6,11 @@
use Sonata\AdminBundle\Form\FormMapper;

/**
* Admin extension to add publish workflow fields.
* Admin extension to add publish workflow time period fields.
*
* @author Daniel Leech <daniel@dantleech.com>
*/
class PublishWorkflowExtension extends AdminExtension
class PublishTimePeriodExtension extends AdminExtension
{
public function configureFormFields(FormMapper $formMapper)
{
Expand All @@ -21,10 +21,6 @@ public function configureFormFields(FormMapper $formMapper)

$formMapper->with('form.group_publish_workflow', array(
'translation_domain' => 'CmfCoreBundle'
))
->add('publishable', 'checkbox', array(
'required' => false,
), array(
))
->add('publish_start_date', 'date', $dateOptions, array(
'help' => 'form.help_publish_start_date',
Expand Down
26 changes: 26 additions & 0 deletions Admin/Extension/PublishableExtension.php
@@ -0,0 +1,26 @@
<?php

namespace Symfony\Cmf\Bundle\CoreBundle\Admin\Extension;

use Sonata\AdminBundle\Admin\AdminExtension;
use Sonata\AdminBundle\Form\FormMapper;

/**
* Admin extension to add publish workflow publishable field.
*
* @author Daniel Leech <daniel@dantleech.com>
*/
class PublishableExtension extends AdminExtension
{
public function configureFormFields(FormMapper $formMapper)
{
$formMapper->with('form.group_publish_workflow', array(
'translation_domain' => 'CmfCoreBundle'
))
->add('publishable', 'checkbox', array(
'required' => false,
), array(
))
->end();
}
}
17 changes: 17 additions & 0 deletions CHANGELOG.md
@@ -1,6 +1,23 @@
Changelog
=========

* **2013-06-20**: [PublishWorkflow] The PublishWorkflowChecker now implements
SecurityContextInterface and the individual checks are moved to voters.
Use the service cmf_core.publish_workflow.checker and call
`isGranted('VIEW', $content)` - or `'VIEW_ANONYMOUS'` if you don't want to
see unpublished content even if the current user is allowed to see it.
Configuration was adjusted: The parameter for the role that may see unpublished
content moved from `role` to `publish_workflow.view_non_published_role`.
The security context is also triggered by a core security voter, so that
using the isGranted method of the standard security will check for
publication.
The PublishWorkflowInterface is split into the reading interfaces
PublishableInterface and PublishTimePeriodInterface as well as
PublishableWriteInterface and PublishableTimePeriodWriteInterface. The sonata
admin extension has been split accordingly and there are now
cmf_core.admin_extension.publish_workflow.time_period and
cmf_core.admin_extension.publish_workflow.publishable.

* **2013-05-16**: [PublishWorkFlowChecker] Removed Request argument
from check method. Class now accepts a DateTime object to
optionally "set" the current time.
2 changes: 2 additions & 0 deletions CmfCoreBundle.php
Expand Up @@ -5,12 +5,14 @@
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Cmf\Bundle\CoreBundle\DependencyInjection\Compiler\RequestAwarePass;
use Symfony\Cmf\Bundle\CoreBundle\DependencyInjection\Compiler\AddPublishedVotersPass;

class CmfCoreBundle extends Bundle
{
public function build(ContainerBuilder $container)
{
parent::build($container);
$container->addCompilerPass(new RequestAwarePass());
$container->addCompilerPass(new AddPublishedVotersPass());
}
}
22 changes: 18 additions & 4 deletions DependencyInjection/CmfCoreExtension.php
Expand Up @@ -17,13 +17,27 @@ public function load(array $configs, ContainerBuilder $container)
$loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('services.xml');

$container->setParameter($this->getAlias().'.role', $config['role']);
$container->setParameter($this->getAlias() . '.document_manager_name', $config['document_manager_name']);

if (!$config['publish_workflow_listener']) {
$container->removeDefinition($this->getAlias() . '.publish_workflow_listener');
if ($config['publish_workflow']['enabled']) {
$this->loadPublishWorkflow($config['publish_workflow'], $loader, $container);
}
}

public function loadPublishWorkflow($config, XmlFileLoader $loader, ContainerBuilder $container)
{
$container->setParameter($this->getAlias().'.publish_workflow.view_non_published_role', $config['view_non_published_role']);
$loader->load('publish_workflow.xml');

if (!$config['request_listener']) {
$container->removeDefinition($this->getAlias() . '.publish_workflow.request_listener');
} elseif (!class_exists('Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter')) {
throw new InvalidConfigurationException("The 'publish_workflow_listener' may not be enabled unless 'Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter' is available.");
throw new InvalidConfigurationException('The "publish_workflow.request_listener" may not be enabled unless "Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter" is available.');
}

if (!class_exists('Sonata\AdminBundle\Admin\AdminExtension')) {
$container->removeDefinition($this->getAlias() . '.admin_extension.publish_workflow.publishable');
$container->removeDefinition($this->getAlias() . '.admin_extension.publish_workflow.time_period');
}
}

Expand Down
40 changes: 40 additions & 0 deletions DependencyInjection/Compiler/AddPublishedVotersPass.php
@@ -0,0 +1,40 @@
<?php

namespace Symfony\Cmf\Bundle\CoreBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;

/**
* Adds all configured publish voters to the access decision manager used for
* the publish workflow checker.
*
* This is about the same as Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass
*
* @author David Buchmann <mail@davidbu.ch>
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class AddPublishedVotersPass implements CompilerPassInterface
{
/**
* {@inheritDoc}
*/
public function process(ContainerBuilder $container)
{
if (!$container->hasDefinition('cmf_core.publish_workflow.access_decision_manager')) {
return;
}

$voters = new \SplPriorityQueue();
foreach ($container->findTaggedServiceIds('cmf_published_voter') as $id => $attributes) {
$priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
$voters->insert(new Reference($id), $priority);
}

$voters = iterator_to_array($voters);
ksort($voters);

$container->getDefinition('cmf_core.publish_workflow.access_decision_manager')->replaceArgument(0, array_values($voters));
}
}
10 changes: 8 additions & 2 deletions DependencyInjection/Configuration.php
Expand Up @@ -15,8 +15,14 @@ public function getConfigTreeBuilder()
$rootNode
->children()
->scalarNode('document_manager_name')->defaultValue('default')->end()
->scalarNode('role')->defaultValue('IS_AUTHENTICATED_ANONYMOUSLY')->end()
->booleanNode('publish_workflow_listener')->defaultFalse()->end()
->arrayNode('publish_workflow')
Copy link
Member Author

Choose a reason for hiding this comment

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

i wonder if we should not enable the pwf things by default. the model and admin classes do expose the information, so as a clueless user, i would expect it to just work. this is about security, so i would prefer security over performance for the default value.

Copy link
Member

Choose a reason for hiding this comment

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

ok for me.

Copy link
Member Author

Choose a reason for hiding this comment

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

added addDefaultsIfNotSet and an enabled parameter

->addDefaultsIfNotSet()
->children()
->booleanNode('enabled')->defaultTrue()->end()
->scalarNode('view_non_published_role')->defaultValue('ROLE_CAN_VIEW_NON_PUBLISHED')->end()
->booleanNode('request_listener')->defaultTrue()->end()
->end()
->end()
->end()
;

Expand Down
37 changes: 29 additions & 8 deletions EventListener/PublishWorkflowListener.php
Expand Up @@ -7,25 +7,47 @@
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker;

use Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter;
use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowCheckerInterface;

/**
* Makes sure only published routes and content can be accessed
* A request listener that makes sure only published routes and content can be
* accessed.
*
* @author David Buchmann <mail@davidbu.ch>
*/
class PublishWorkflowListener implements EventSubscriberInterface
{
/**
* @var PublishWorkflowCheckerInterface
* @var PublishWorkflowChecker
*/
protected $publishWorkflowChecker;

/**
* @param PublishWorkflowCheckerInterface $publishWorkflowChecker
* The attribute to check with the workflow checker, typically VIEW or VIEW_ANONYMOUS
*
* @var string
*/
private $attribute;

/**
* @param PublishWorkflowChecker $publishWorkflowChecker
* @param string $attribute the attribute name to check
*/
public function __construct(PublishWorkflowCheckerInterface $publishWorkflowChecker)
public function __construct(PublishWorkflowChecker $publishWorkflowChecker, $attribute = 'VIEW')
{
$this->publishWorkflowChecker = $publishWorkflowChecker;
$this->attribute = $attribute;
}

public function getAttribute()
{
return $this->attribute;
}
public function setAttribute($attribute)
{
$this->attribute = $attribute;
}

/**
Expand All @@ -38,12 +60,12 @@ public function onKernelRequest(GetResponseEvent $event)
$request = $event->getRequest();

$route = $request->attributes->get(DynamicRouter::ROUTE_KEY);
if ($route && !$this->publishWorkflowChecker->checkIsPublished($route, false, $request)) {
if ($route && !$this->publishWorkflowChecker->isGranted($this->getAttribute(), $route)) {
throw new NotFoundHttpException('Route not found at: ' . $request->getPathInfo());
}

$content = $request->attributes->get(DynamicRouter::CONTENT_KEY);
if ($content && !$this->publishWorkflowChecker->checkIsPublished($content, false, $request)) {
if ($content && !$this->publishWorkflowChecker->isGranted($this->getAttribute(), $content)) {
throw new NotFoundHttpException('Content not found for: ' . $request->getPathInfo());
}
}
Expand All @@ -59,5 +81,4 @@ static public function getSubscribedEvents()
KernelEvents::REQUEST => array(array('onKernelRequest', 1)),
);
}

}
32 changes: 32 additions & 0 deletions PublishWorkflow/PublishTimePeriodInterface.php
@@ -0,0 +1,32 @@
<?php

namespace Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow;

/**
* Interface for time period based publish checking.
*
* Both start and end date are optional, with null being interpreted as always
* started resp. never ending.
*/
interface PublishTimePeriodInterface
{
/**
* Return the date from which the content should be published.
*
* A NULL value is interpreted as a date in the past, meaning the content
* is publishable unless publish end date is set and in the past.
*
* @return \DateTime|null
*/
public function getPublishStartDate();

/**
* Return the date at which the content should stop being published.
*
* A NULL value is interpreted as saying that the document will
* never end being publishable.
*
* @return \DateTime|null
*/
public function getPublishEndDate();
}
31 changes: 31 additions & 0 deletions PublishWorkflow/PublishTimePeriodWriteInterface.php
@@ -0,0 +1,31 @@
<?php

namespace Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow;

/**
* Interface to expose editable time period publishing information.
*/
interface PublishTimePeriodWriteInterface extends PublishTimePeriodInterface
{
/**
* Set the date from which the content should
* be considered publishable.
*
* Setting a NULL value asserts that the content
* has always been publishable.
*
* @param \DateTime|null $publishDate
*/
public function setPublishStartDate(\DateTime $publishDate = null);

/**
* Set the date at which the content should
* stop being published.
*
* Setting a NULL value asserts that the
* content will always be publishable.
*
* @param \DateTime|null $publishDate
*/
public function setPublishEndDate(\DateTime $publishDate = null);
}