Skip to content

WebSecurityConfiguration should autowire PermissionEvaluator the same way GlobalMethodSecurityConfiguration does #4077

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
candrews opened this issue Sep 29, 2016 · 9 comments
Closed

WebSecurityConfiguration should autowire PermissionEvaluator the same way GlobalMethodSecurityConfiguration does #4077

candrews opened this issue Sep 29, 2016 · 9 comments
Assignees
@rwinch
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Milestone
5.0.0.M5

Comments

@candrews
Copy link
Contributor

GlobalMethodSecurityConfiguration autowires PermissionEvaluator from the context:
https://github.com/spring-projects/spring-security/blob/4.1.3.RELEASE/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java#L154

WebSecurityConfiguration should do the same thing.

Currently, it's surprising that when a PermissionEvaluator is set up, it just works (with no configuration other than declaring the PermissionEvaluator bean) when used from Java annotations but the same expression always returns denied (as that's what the default configuration does) when used from in a web context (such as in a JSP sec: expression).
@rwinch
Copy link
Member

rwinch commented Sep 30, 2016

Thanks for creating the issue! Would you be interested in submitting a PR?

@rwinch rwinch added the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Sep 30, 2016
@candrews candrews mentioned this issue Nov 1, 2016
Merged
@candrews
Copy link
Contributor Author

candrews commented Nov 1, 2016

@rwinch PR submitted. Thanks in advance for working with me on this issue.

@candrews
Copy link
Contributor Author

I've updated the PR to include testing.

Note that I've followed the style/approach of #4020

@rwinch rwinch added type: enhancement A general enhancement in: config An issue in spring-security-config and removed status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Nov 10, 2016
@rwinch rwinch added this to the 5.0.0.M1 milestone Nov 10, 2016
@rwinch
Copy link
Member

rwinch commented Nov 10, 2016

@candrews Thanks!

candrews added a commit to candrews/spring-security that referenced this issue Feb 8, 2017
@candrews
Configure permissionEvaluator and roleHierarchy by default
023e8fa 
Implementations of AbstractSecurityExpressionHandler (such as the very commonly used DefaultWebSecurityExpressionHandler) get PermissionEvaluator and RoleHierarchy from the application context (if the application context is provided, and exactly one of such a bean exists in it). This approach matches that used in GlobalMethodSecurityConfiguration, making everything in Spring Security work the same way (including WebSecurity).

Issue spring-projectsgh-4077
@rwinch rwinch modified the milestones: 5.0.0.M1, 5.0.0.M2 May 10, 2017
@rwinch rwinch modified the milestones: 5.0.0.M2, 5.0.0.M3 Jun 15, 2017
@jgrandja jgrandja modified the milestones: 5.0.0.M3, 5.0.0.M4 Jul 24, 2017
@rwinch rwinch modified the milestones: 5.0.0.M4, 5.0.0.M5 Sep 13, 2017
rwinch pushed a commit that referenced this issue Sep 18, 2017
@candrews @rwinch
Configure permissionEvaluator and roleHierarchy by default
3bf6bf1 
Implementations of AbstractSecurityExpressionHandler (such as the very commonly used DefaultWebSecurityExpressionHandler) get PermissionEvaluator and RoleHierarchy from the application context (if the application context is provided, and exactly one of such a bean exists in it). This approach matches that used in GlobalMethodSecurityConfiguration, making everything in Spring Security work the same way (including WebSecurity).

Issue gh-4077
rwinch added a commit that referenced this issue Sep 18, 2017
@rwinch
Polish PermissionEvaluator Autowired into Web Security
8a66d0c 
Issue gh-4077
@rwinch
Copy link
Member

rwinch commented Sep 18, 2017

Thanks for your work on this! This has been merged via 3bf6bf1 I put polish on it in 8a66d0c so that the actual object is not changed but the configuration is updated. This ensures the logic of looking up beans is kept within Configuration and the Bean is kept just a simple Bean.

@rwinch rwinch closed this as completed Sep 18, 2017
@rwinch rwinch self-assigned this Sep 18, 2017
rwinch pushed a commit that referenced this issue Oct 30, 2017
@candrews @rwinch
Configure permissionEvaluator and roleHierarchy by default
bf075a2 
Implementations of AbstractSecurityExpressionHandler (such as the very commonly used DefaultWebSecurityExpressionHandler) get PermissionEvaluator and RoleHierarchy from the application context (if the application context is provided, and exactly one of such a bean exists in it). This approach matches that used in GlobalMethodSecurityConfiguration, making everything in Spring Security work the same way (including WebSecurity).

Issue gh-4077
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
@candrews @thomasdarimont
Configure permissionEvaluator and roleHierarchy by default
31ecf94 
Implementations of AbstractSecurityExpressionHandler (such as the very commonly used DefaultWebSecurityExpressionHandler) get PermissionEvaluator and RoleHierarchy from the application context (if the application context is provided, and exactly one of such a bean exists in it). This approach matches that used in GlobalMethodSecurityConfiguration, making everything in Spring Security work the same way (including WebSecurity).

Issue spring-projectsgh-4077
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
@rwinch @thomasdarimont
Polish PermissionEvaluator Autowired into Web Security
6b7c675 
Issue spring-projectsgh-4077
@candrews
Copy link
Contributor Author

candrews commented Apr 29, 2018 •
edited
Loading

The polish broke one of the use cases.

Using thymeleaf spring security extras, you can conditionally show elements like this:
<div sec:authorize="hasPermission(#vars.study,'DELETE')">

The DefaultWebSecurityExpressionHandler that ends up being used, with your polish, has a type of org.springframework.security.access.expression.DenyAllPermissionEvaluator instead of the permission evaluator.

@rwinch
Copy link
Member

rwinch commented Apr 30, 2018

@candrews This issue is closed. Can you please create a new ticket with details of the issue you are experiencing?

@candrews candrews mentioned this issue Apr 30, 2018
Closed
@candrews
Copy link
Contributor Author

@candrews This issue is closed. Can you please create a new ticket with details of the issue you are experiencing?

I was thinking this issue should be reopened, as the reported issue was not fixed. :-)

@rwinch
Copy link
Member

rwinch commented Apr 30, 2018

Thanks for creating the new ticket. I can understand why you might have thought that reopening this was what needed to be done. To clarify...Once a ticket is associated with a release and that release is performed we do not reopen the issue because the change log is considered immutable at that point. Instead we create a new ticket. Thanks again!

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

@rwinch rwinch

Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
None yet
Milestone
5.0.0.M5
Development

No branches or pull requests
3 participants
@candrews @rwinch @jgrandja