-
Notifications
You must be signed in to change notification settings - Fork 33
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
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
991a8d0
refactor publish workflow to use security, cleanup configuration
dbu 6c7aada
refactoring to have a separate check
dbu 65ccec2
cleanup after feedback
dbu e4e1275
fix tests and cleanup issues
dbu bdf22d7
adding sequence info to xml schema
dbu e750115
Merge remote-tracking branch 'origin/master' into workflow-security
dbu c7a67ae
totally refactored the handling for depth in the CmfHelper
lsmith77 5afa058
Merge pull request #67 from symfony-cmf/workflow-security-test-fixes
lsmith77 87fd7f3
Refactored to inject helper rather than extending it.
dantleech 45f1fba
Fixed service.xml and added functional test
dantleech 4587c0c
Removed extra methods by extending Twig_Extension
dantleech 93fb382
Merge pull request #68 from dantleech/inject-helper
dbu 003d8cd
split workflow editing interfaces
dbu 89bf726
Merge remote-tracking branch 'origin/master' into workflow-security
dbu e0d41d8
adjust the extension test
dbu b516cd7
doc cleanups
dbu fc927c9
adjust test to fixed testing component
dbu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 <[email protected]> | ||
*/ | ||
class PublishWorkflowExtension extends AdminExtension | ||
class PublishTimePeriodExtension extends AdminExtension | ||
{ | ||
public function configureFormFields(FormMapper $formMapper) | ||
{ | ||
|
@@ -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', | ||
|
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <[email protected]> | ||
*/ | ||
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(); | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <[email protected]> | ||
* @author Johannes M. Schmitt <[email protected]> | ||
*/ | ||
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)); | ||
} | ||
} |
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 <[email protected]> | ||
*/ | ||
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; | ||
} | ||
|
||
/** | ||
|
@@ -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()); | ||
} | ||
} | ||
|
@@ -59,5 +81,4 @@ static public function getSubscribedEvents() | |
KernelEvents::REQUEST => array(array('onKernelRequest', 1)), | ||
); | ||
} | ||
|
||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok for me.
There was a problem hiding this comment.
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