Skip to content

Conversation

@Metavirulent
Copy link
Contributor

SEC-2342
Extracts the mask comparison in DefaultPermissionGrantingStrategy into a protected method.
This allows overriding the default behavior in a subclass without having to copy the whole isGranted() method.
As discussed in the above issue (and others like SEC-1140), users can choose to implement a different strategy, e.g. do a bitwise mask comparison instead of the integer comparison.

Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please see inline comments.

* @param p the Permission we are checking against.
* @return true, if the respective masks are considered to be equal.
*/
protected boolean comparePermissionMasks(AccessControlEntry ace, Permission p) {
Copy link
Member

Choose a reason for hiding this comment

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

This method is very implementation specific and would not make sense if a user decides to change the strategy. Please rename to isGranted.

Copy link
Member

Choose a reason for hiding this comment

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

Please add a test to demonstrate overriding this method works

@rwinch rwinch self-assigned this Oct 3, 2018
@rwinch rwinch added this to the 5.1.1 milestone Oct 3, 2018
@rwinch rwinch added in: core An issue in spring-security-core type: enhancement A general enhancement labels Oct 3, 2018
@rwinch rwinch modified the milestones: 5.1.1, 5.2.x Oct 15, 2018
@rwinch
Copy link
Member

rwinch commented Oct 15, 2018

Moved to 5.2.x since it is an enhancement

@rwinch rwinch removed their assignment Jul 29, 2019
@eleftherias eleftherias self-assigned this Oct 21, 2019
@eleftherias eleftherias merged commit 2cc0555 into spring-projects:master Oct 21, 2019
@eleftherias
Copy link
Contributor

Thanks for the PR @Metavirulent! This is now merged into master.

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

Labels

in: core An issue in spring-security-core type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants