Skip to content

@EnableMethodSecurity does not resolve @PreAuthorize on interfaces #11175

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

Closed
noelbundick-msft opened this issue Apr 28, 2022 · 2 comments · Fixed by #11177
Closed

@EnableMethodSecurity does not resolve @PreAuthorize on interfaces #11175

noelbundick-msft opened this issue Apr 28, 2022 · 2 comments · Fixed by #11177
Assignees
Labels
status: duplicate A duplicate of another issue type: bug A general bug

Comments

@noelbundick-msft
Copy link

Describe the bug
When using @EnableGlobalMethodSecurity(prePostEnabled = true) alongside Spring Data REST, it is possible to add @PreAuthorize("hasRole('SOMETHING')") on the repository interface, which secures the entire repository. You can also use @PreAuthorize on individual methods

When using the newer @EnableMethodSecurity attribute, @PreAuthorize only works on individual methods, and does not work on an interface. This is a change in behavior that could result in potential accidental data leakage when upgrading to the latest Spring Security bits.

To Reproduce

  • Wire up Spring Data REST in a project with @EnableGlobalMethodSecurity(prePostEnabled = true)
  • Add @PreAuthorize("hasRole('BOGUS')") to your repository interface
  • Make a GET request, observe that it is rejected
  • Upgrade to @EnableMethodSecurity
  • Make a request, observe that a response is returned instead of rejected. You are now leaking data to unauthorized callers

Expected behavior
@PreAuthorize to be processed the same way as before

Sample

https://github.com/noelbundick-msft/spring-security-methodsecurity-bug

@noelbundick-msft noelbundick-msft added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 28, 2022
@evgeniycheban
Copy link
Contributor

Hi @noelbundick-msft!

I've checked the sample you provided and I see that the findPreAuthorizeAnnotation performs search for @PreAuthorize annotation on the class that declares the called method in this case SimpleJpaRepository instead of $Proxy that implements repository interface.

This is definitely a bug, thanks for reporting.

The code I used to reproduce this:

@EnableMethodSecurity
// @EnableGlobalMethodSecurity(prePostEnabled = true)
@Configuration
public class MethodSecurityConfig {}
@PreAuthorize("hasRole('BOGUS')")
public interface ThingRepository extends JpaRepository<Thing, Long> {
  @Override
  @PreAuthorize("hasRole('BOGUS')")
  Optional<Thing> findById(Long id);
}
Thing testThing = new Thing();
testThing.setName("testName");
this.repository.save(testThing); // Should throw `AccessDeniedException` but it doesn't.

evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue May 3, 2022
…interfaces through a Proxy

Removed proxy unwrapping in case of resolving Method Security annotations,
this cause an issue when interfaces which are implemented by the proxy was skipped,
resulting in a missing security checks on those methods.

Closes spring-projectsgh-11175
rwinch pushed a commit that referenced this issue May 3, 2022
…interfaces through a Proxy

Removed proxy unwrapping in case of resolving Method Security annotations,
this cause an issue when interfaces which are implemented by the proxy was skipped,
resulting in a missing security checks on those methods.

Closes gh-11175
@rwinch rwinch self-assigned this May 3, 2022
@rwinch rwinch added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels May 3, 2022
@rwinch
Copy link
Member

rwinch commented May 3, 2022

Thanks for the report, I closed this in favor of gh-11177

rwinch pushed a commit that referenced this issue May 3, 2022
…interfaces through a Proxy

Removed proxy unwrapping in case of resolving Method Security annotations,
this cause an issue when interfaces which are implemented by the proxy was skipped,
resulting in a missing security checks on those methods.

Closes gh-11175
rwinch pushed a commit that referenced this issue May 3, 2022
…interfaces through a Proxy

Removed proxy unwrapping in case of resolving Method Security annotations,
this cause an issue when interfaces which are implemented by the proxy was skipped,
resulting in a missing security checks on those methods.

Closes gh-11175
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue type: bug A general bug
Projects
None yet
3 participants