-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle][Workflow] Add a way to register a guard expression in the configuration #21935
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
Conversation
third option: do as the SecurityListener in FrameworkExtraBundle: allow expressions, but not by using the ExpressionVoter itself, allowing you to expose Pros:
I see several cons in your option 2:
|
@stof Thanks. That's very cool. I did not think to do that ;) I will do that ! Just to clarify your point about 2/: I said it's more flexible because in the end you write pure PHP. That's why it is the most flexible solution. Thus you have access to the object in the voter (because the listener pass it to the AuthChecker) |
@lyrixx but forcing to pass it is not always the most flexible. I have voters which explicitly vote on |
d3433b5
to
dadae1c
Compare
I pushed the new version ;) Ready for review. |
More of a semantic, but why 'guard' instead of 'authorization'? |
@iltar usually people use refs:
Thus, here you can block a transition not only with the security but with, for example, the subject itself. So authorization seems too "security related" to me. |
The expression seems to be used for authorization, hence I was wondering. Guard sounds fine in this case though. |
I updated the PR description to add another example where we don't use the security. I guess it's better now. |
In routing, this is called "condition", maybe better and more consistent? |
In that case, it will not be consistent with the workflow. to me, |
dadae1c
to
09efa63
Compare
Fabbot is red, and deps=low line also |
e3b0355
to
e00d2f4
Compare
…in the configuration
e00d2f4
to
ab3b12d
Compare
@nicolas-grekas Rebase + Fixed tests. Now it's green. @stof Is it what you had in mind ? |
👍 |
Thank you @lyrixx. |
…ard expression in the configuration (lyrixx) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle][Workflow] Add a way to register a guard expression in the configuration | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - --- Many people already asked for this feature so ... here we go 🎉 --- Usage: ```yml transitions: journalist_approval: guard: "is_fully_authenticated() and has_role('ROLE_JOURNALIST') or is_granted('POST_EDIT', subject)" from: wait_for_journalist to: approved_by_journalist publish: guard: "subject.isPublic()" from: approved_by_journalist to: published ``` Commits ------- ab3b12d [FrameworkBundle][Workflow] Add a way to register a guard expression in the configuration
This functionality (and also other extensions) was implemented in https://github.com/GlobalTradingTechnologies/workflow-extensions-bundle long time ago) See for example transition blocking feature |
This PR was merged into the 3.3-dev branch. Discussion ---------- [Workflow] sync the changelog | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20751, #21334, #21933, #21935, #21950 | License | MIT | Doc PR | Commits ------- 98a18ee [Workflow] sync the changelog
What about adding guard expressions for workflow.guard and workflow.id.guard events too? |
Hello @destillat you are commenting a closed/merged PR. So I suggest you to open a new issue instead. Thanks. |
FYI this is not documented |
you are commenting a closed/merged PR. |
Many people already asked for this feature so ... here we go 🎉
Usage: