Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Security] Continuation : Add messages on votes #59573

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
Closed
wants to merge 1 commit into from
Closed

[Security] Continuation : Add messages on votes #59573

wants to merge 1 commit into from

Conversation

eltharin
Copy link
Contributor

@eltharin eltharin commented Jan 21, 2025 •
edited
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #27995, #26343, #35592, #43147
License MIT

This PR continue the work started by @maidmaid and @noniagriconomie in #35592, continued by @yellow1912 in #43147 continued by @alamirault in #46493.

I rebase on 7.2 and remove deprecation as suggested by nicolas-grekas for not causing backward compatiblity breaks.

It allow to have informations about AccessDecisionManager and Voters (And understand why we have an AccessDeniedException with clear infos).
170878112-f93f8d90-97b7-42f8-8494-603d6b2019e2

After #58107, I take the work from the beginning so I open a new PR.

@eltharin eltharin requested a review from chalasr as a code owner January 21, 2025 20:18
@carsonbot carsonbot added this to the 7.3 milestone Jan 21, 2025
@eltharin eltharin changed the title add capability to answer VoteObject Continuation : Add messages on votes Jan 21, 2025
@eltharin eltharin mentioned this pull request Jan 21, 2025
Closed
@eltharin eltharin force-pushed the voteObjects branch 5 times, most recently from c2c93f2 to 41259dd Compare January 21, 2025 21:19
@carsonbot carsonbot changed the title Continuation : Add messages on votes [Security] Continuation : Add messages on votes Jan 21, 2025
@eltharin eltharin force-pushed the voteObjects branch 2 times, most recently from 126b96b to e8186ac Compare January 21, 2025 21:33
@eltharin eltharin force-pushed the voteObjects branch 5 times, most recently from 400a646 to ca672e7 Compare January 29, 2025 14:27
@nicolas-grekas
Copy link
Member

I'm losing track of the PR I should review.
Please keep one PR and stick to it.
From what I can see, it might be better to continue #58107: this one is a BC break, so a dead end.

@eltharin
Copy link
Contributor Author

OK I will merge that on other.
But where do you have a BC? I try with a new/old voter, new/old accesmanager, new/old strategy.
the only BC I think have done is for VoteEvent whitch is internal.
but maybe i miss somewhere...

@nicolas-grekas
Copy link
Member

Changing the return type of a method is a BC break (callers may get results they don't expect.)

@eltharin eltharin closed this Feb 10, 2025
@eltharin
Copy link
Contributor Author

As you ask, I move all to previous PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

Assignees
No one assigned
Labels
Feature Security Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Security][DX] Be able to know why exactly SecurityVoter returns false
4 participants
@eltharin @nicolas-grekas @GromNaN @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.