Skip to content

SecuredAuthorizationManager should cache annotation's value #12335

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

Merged
merged 2 commits into from
Jan 5, 2023

Conversation

evgeniycheban
Copy link
Contributor

Closes gh-12232

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks, @evgeniycheban! This looks great. I left a piece of feedback inline.

Also, now that we are in 2023, will you please update the copyright year for these files?

@@ -74,14 +74,14 @@ void checkWhenUserHasNotAnyAuthorityThenDeniedDecision() {
}

@Test
void hasRoleWhenRoleHierarchySetThenGreaterRoleTakesPrecedence() {
void checkWhenRoleHierarchySetThenGreaterRoleTakesPrecedence() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you please put this in a separate commit with a comment that describes why the change was made? By doing that, each commit will stay focused on one atomic change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jzheaux jzheaux added status: duplicate A duplicate of another issue in: core An issue in spring-security-core type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 4, 2023
@jzheaux jzheaux added this to the 6.1.0-M1 milestone Jan 4, 2023
- Update copyright header
- Use Set.of instead of HashSet in AuthorityAuthorizationManager
- Align roleHierarchy test name with other tests in AuthoritiesAuthorizationManagerTests
@jzheaux jzheaux merged commit 1bbbd04 into spring-projects:main Jan 5, 2023
@jzheaux
Copy link
Contributor

jzheaux commented Jan 5, 2023

Thanks, @evgeniycheban! This is now merged into main.

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 status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SecuredAuthorizationManager should cache annotation's value
3 participants