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.

Sign up for GitHub

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

Jump to bottom

Rewrite Blacklist Entry without Voters #5084

Closed
weaverryan opened this issue Mar 15, 2015 · 2 comments
Closed

Rewrite Blacklist Entry without Voters #5084

weaverryan opened this issue Mar 15, 2015 · 2 comments
Labels
actionable Clear and specific issues ready for anyone to take them. hasPR A Pull Request has already been submitted for this issue. Security

Comments

@weaverryan
Copy link
Member

This article http://symfony.com/doc/current/cookbook/security/voters.html needs to be rewritten using a simple event listener. Using a voter is awkward - see #4304 for the conversation.
@weaverryan weaverryan added actionable Clear and specific issues ready for anyone to take them. Security labels Mar 15, 2015
@stof
Copy link
Member

stof commented Mar 16, 2015

Well, this article is not about blacklisting entries but about explaining how to write voters (see the URL). So the proper fix would be to rewrite the chapter to use a better example

@weaverryan
Copy link
Member Author

@stof But we have the other voter article now, which is much better: http://symfony.com/doc/current/cookbook/security/voters_data_permission.html. So perhaps the best option is to remove the blacklisting one entirely. A quick scan shows me that it doesn't add anything unique, except for the Access Decision Strategy part, which could be moved to the other one.

@wouterj wouterj mentioned this issue Jun 27, 2015
Merged
@wouterj wouterj added the hasPR A Pull Request has already been submitted for this issue. label Jun 28, 2015
weaverryan added a commit that referenced this issue Jul 16, 2015
@weaverryan
feature #5453 Cleanup security voters cookbook recipes (WouterJ)
589828d 
This PR was merged into the 2.3 branch.

Discussion
----------

Cleanup security voters cookbook recipes

| Q | A
| --- | ---
| Doc fix? | yes
| New docs? | no
| Applies to | all
| Fixed tickets | #5084

Blacklisting using voters isn't a good practice and in fact, the article (`cookbook/security/voters`) was almost completely equal to the much newer voter article (`cookbook/security/voters_data_permission`). I've moved the only different piece (access decision strategy) to the newer voter article and then removed the old voter article.

As the old voter article had a much nicer URL than the new one, I then renamed the new article to have the better name and set up a redirection. A [quick search on Google](https://www.google.nl/search?q=%5Blink%3Ahttp%3A%2F%2Fsymfony.com%2Fdoc%2Fcurrent%2Fcookbook%2Fsecurity%2Fvoters.html%5D+-site%3Ahttp%3A%2F%2Fsymfony.com) revealed that there are almost no links to the old article talking about blacklisting specifically, just about voters. So I think we're good here.

The diff is horrible, that's why I've tried to create multiple commits (this will ease reviewing process). The only commit requiring review is 9c169c7 (as the other 2 are just moving and removing stuff).

Commits
-------

93484a7 Remove the old voter article
9c169c7 Rewrite new section
acf66f9 Move access decision strategy section
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
actionable Clear and specific issues ready for anyone to take them. hasPR A Pull Request has already been submitted for this issue. Security
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@weaverryan @stof @wouterj