adjust to workflow moved to security #25
Conversation
|
as discovered in the core bundle, this will fail if there is no firewall configured. we could also inject the workflow service from core, but only if it is enabled. we probably need something here to make the check optional. |
| ViewHandlerInterface $viewHandler = null, | ||
| DocumentManager $dm, | ||
| PublishWorkflowCheckerInterface $pwfc | ||
| SecurityContextInterface $securityContext |
There was a problem hiding this comment.
should we allow null here and skip the check if no security context is defined?
There was a problem hiding this comment.
If this were optional I would prefer it to be a setter. Though, I'm +0 on making SecurityContext optional, in I think the security component is inevitable in most cases.
There was a problem hiding this comment.
yeah. i created symfony-cmf/core-bundle#69 - what
do you think about that, would this be a solution?
There was a problem hiding this comment.
I think makes some sense in terms of cleaner code, but doesn't help with getting rid of dependencies.
There was a problem hiding this comment.
But I am +0 for dependencies here, and probably +1 for the PWFAlways checker, although I have never used anything like that so not really in a position to express a strong opinion.
|
this is adjusted to the now merged CoreBundle. @dantleech would be great if you can review this until tomorrow so we can tag the release |
| { | ||
| $post = $contentDocument; | ||
|
|
||
| if (true !== $this->pwfc->checkIsPublished($post)) { | ||
| if (true !== $this->securityContext->isGranted('VIEW', $post)) { |
There was a problem hiding this comment.
I wonder if VIEW is the best name. VIEW_UNPUBLISHED?
There was a problem hiding this comment.
that name is part of the pr on core. VIEW is the right thing because
that plays well with checks if a user is allowed to do the VIEW
operation. we do not check the permission if the user is allowed to VIEW
unpublished documents, just if he is allowed to VIEW this document.
everything else is handled in the publish workflow checker.
adjust to workflow moved to security
Adjust to symfony-cmf/core-bundle#59