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

Use IsGranted in EntryController #7375

Merged
merged 2 commits into from Apr 4, 2024

Conversation

yguedidi
Copy link
Contributor

Better reviewed commit by commit.
Left as a draft PR so that it's not accidentaly merged before squashing in one commit after approve :)

For EntryVoter, all attributes are using the same rule: being the user of the entry.
But because it's about security, I prefered to be as granular as possible, in case each actions grant rules change in the future.
That's why I added an attribute per action, and updated the templates accordingly.

Same for MainVoter, that will own general actions not on a particular subject.
Just used the same LIST_ENTRIES for all entries listing actions as each is just a specific filtering.

@yguedidi yguedidi added this to the 2.7.0 milestone Mar 23, 2024
@yguedidi yguedidi force-pushed the use-isgranted-in-entrycontroller branch 2 times, most recently from bb23764 to ca7ee81 Compare April 4, 2024 04:56
Copy link
Member

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@yguedidi
Copy link
Contributor Author

yguedidi commented Apr 4, 2024

Thanks @j0k3r! squashing to a single commit before merging

@yguedidi yguedidi force-pushed the use-isgranted-in-entrycontroller branch from ca7ee81 to e66ea21 Compare April 4, 2024 08:25
@yguedidi yguedidi marked this pull request as ready for review April 4, 2024 08:26
@yguedidi yguedidi enabled auto-merge April 4, 2024 08:26
@yguedidi yguedidi requested a review from j0k3r April 4, 2024 08:29
@yguedidi yguedidi merged commit 96cb024 into wallabag:master Apr 4, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants