Skip to content
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

(Spring Boot 2.7->3.2) Duplicate @PreAuthorize annotation error across class hierarchy #15097

Closed
arnaldop opened this issue May 19, 2024 · 10 comments
Assignees
Labels
in: core An issue in spring-security-core status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@arnaldop
Copy link

Describe the bug
I have an abstract class that has the @PreAuthorize annotation. Its subclass also has an identical @PreAuthorize annotation.

To Reproduce
Attempting to invoke an endpoint in the subclass results in this error message:

org.springframework.core.annotation.AnnotationConfigurationException: Found more than one annotation of type interface org.springframework.security.access.prepost.PreAuthorize attributed to class com.agencycomp.report.ReportController Please remove the duplicate annotations and publish a bean to handle your authorization logic.

Expected behavior
In Spring Boot 2.7.3, this code worked as is. (org.springframework.security:spring-security-core:jar:5.7.11:compile)
After migrating to Spring Boot 3.2, this no longer works. (org.springframework.security:spring-security-core:jar:6.2.4:compile)

I was able to remove exact duplicates, but as the code sample below reveals, there are places there the SpEL is not the same, so they should not be considered duplicated.

Ideally, I should be able to define the @PreAuthorize annotation in the superclass, and only override it as needed in subclasses. This is how it worked previously.

Sample

@PreAuthorize("!principal.locked")
public abstract class UserDependentController {
    @PostMapping
    protected Object create(@NonNull @Valid @RequestBody final Object dto) {
        return null;
    }
}

@RestController
@RequestMapping("app/reports")
@PreAuthorize("!principal.locked && hasRole('ROLE_REGULAR')")
//@PreAuthorize("hasRole('ROLE_REGULAR')") -- attempt to create an annotation that is not the same
class ReportController extends UserDependentController {
    @GetMapping("types")
    Page<Object> getTypes() {
        return null;
    }
}
@arnaldop arnaldop added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels May 19, 2024
@jzheaux
Copy link
Contributor

jzheaux commented May 20, 2024

Hi, @arnaldop, thanks for the suggestion. Can you please clarify what in your sample you want to be able to do? I can't tell if you want to be able to uncomment the commented code, if you want to be able to replace the commented line with the uncommented one, etc.

@jzheaux jzheaux added in: core An issue in spring-security-core type: enhancement A general enhancement status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels May 20, 2024
@arnaldop
Copy link
Author

arnaldop commented May 21, 2024

@jzheaux, as far as my code sample above, the uncommented version is how it worked in Spring Boot 2.7. The methods in ReportController had the additional hasRole('ROLE_REGULAR') clause. But now, I can't have @PreAuthorize in the parent and in the subclass. So my options would be to:

  1. Annotate every single method in ReportController with @PreAuthorize("!principal.locked && hasRole('ROLE_REGULAR')"), which is a lot of duplication.
  2. Remove the annotation from UserDependentController and add @PreAuthorize("!principal.locked") to every single one of the extending classes, which is a lot of duplication.

Instead, I want to be able to define overriding @PreAuthorize annotations to subclasses and methods as needed.

An example of this in Spring Data is the @org.springframework.data.repository.NoRepositoryBean annotation. You would add it to a parent repository interface, to signal the framework that that class should not be instantiated as Bean. Then you implement that interface and annotate the children with @org.springframework.stereotype.Repository, which will be instantiated.

Considering the Spring Security test class org.springframework.security.authorization.method.AuthorizationAnnotationUtilsTests, I would like to have the following additional classes, all of them allowed, and with the given results.

@PreAuthorize("hasRole('someRole')")
private static abstract class PreAuthorizeAnnotationsOnAbstractClass {
	public void someMethod1() { } // allowed for ROLE_SOME_ROLE
}

@PreAuthorize("hasRole('otherRole')")
private static class PreAuthorizeAnnotationsOnSubClassFirst extends PreAuthorizeAnnotationsOnAbstractClass {
	public void someMethod2() { } // allowed for ROLE_OTHER_ROLE
}

@PreAuthorize("hasRole('someRole') || hasRole('otherRole')")
private static class PreAuthorizeAnnotationsOnSubClassSecond extends PreAuthorizeAnnotationsOnAbstractClass {
	public void someMethod3() { } // allowed for ROLE_SOME_ROLE or ROLE_OTHER_ROLE
}

private static class PreAuthorizeAnnotationsOnSubClassThird extends PreAuthorizeAnnotationsOnAbstractClass {
	@PreAuthorize("hasRole('anotherRole')")
	public void someMethod4() { } // allowed for ROLE_ANOTHER_ROLE
}

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 21, 2024
@kse-music
Copy link
Contributor

If you use v6.3.0,you can Using Meta Annotations as below:

@Target({ ElementType.METHOD, ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@PreAuthorize("!principal.locked && hasRole('{value}')")
public @interface HasRole{
      String value();
}


@RestController
@RequestMapping("app/reports")
@HasRole("ROLE_REGULAR")
class ReportController extends UserDependentController {
    @GetMapping("types")
    Page<Object> getTypes() {
        return null;
    }
}

@jzheaux jzheaux self-assigned this May 28, 2024
@arnaldop
Copy link
Author

@kse-music, your proposed solution did not work, at least with Spring Security 6.2.4 (dependency from Spring Boot 3.2).

And it makes sense. All that we did with the meta-annotation was to move the duplicate @PreAuthorize to a different place, but it's still a duplicated @PreAuthorize. Unless I am misunderstanding something major.

Here is my meta-annotation:

@Target({ ElementType.METHOD, ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@PreAuthorize("!principal.locked && hasRole('ROLE_REGULAR')")
public @interface MetaPreAuthorize {
}

Here is the code using the meta-annotation:

@RequiredArgsConstructor
@RestController
@RequestMapping("app/reports")
@MetaPreAuthorize
@Tag(name = "Reports")
class ReportController extends UserDependentController<Report, ReportDto>
        implements SearchableController<Report, ReportDto, ReportSearchDto> {
    @Getter
    private final ReportService service;

    @GetMapping("types")
    Page<ReportTypeDto> getTypes() {
        return new PageImpl<>(service.getTypes());
    }
}

Here is the error:

2024-05-28T09:19:14.499-04:00 ERROR 52433 --- [mcat-handler-19] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed: org.springframework.core.annotation.AnnotationConfigurationException: Found more than one annotation of type interface org.springframework.security.access.prepost.PreAuthorize attributed to class com.agencycomp.report.ReportController Please remove the duplicate annotations and publish a bean to handle your authorization logic.] with root cause

org.springframework.core.annotation.AnnotationConfigurationException: Found more than one annotation of type interface org.springframework.security.access.prepost.PreAuthorize attributed to class com.agencycomp.report.ReportController Please remove the duplicate annotations and publish a bean to handle your authorization logic.
	at org.springframework.security.authorization.method.AuthorizationAnnotationUtils.findUniqueAnnotation(AuthorizationAnnotationUtils.java:89) ~[spring-security-core-6.2.4.jar:6.2.4]

@kse-music
Copy link
Contributor

kse-music commented May 29, 2024

@arnaldop after moving the PreAuthorize annotation value on UserDependentController to MetaPreAuthorize, you also need to remove the PreAuthorize annotation on parent class and interface.

@arnaldop
Copy link
Author

arnaldop commented May 29, 2024 via email

jzheaux added a commit to jzheaux/spring-security that referenced this issue May 30, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue May 30, 2024
- Now searches methods and classes together in the hierarchical search
instead of first the method hierarchy and then the class hierarchy
- Stops when it finds annotations on a method or class, sees if the
closest annotation is not duplicated.

Closes spring-projectsgh-15097
@jzheaux
Copy link
Contributor

jzheaux commented May 30, 2024

Related to #13234, #13490, and #11436

@anaconda875
Copy link

Hi @jzheaux , when is the fix available? I also face this issue on Spring boot 3.2.0 (Webflux, Security). Thanks

jzheaux added a commit to jzheaux/spring-security that referenced this issue Jul 13, 2024
@jzheaux jzheaux added this to the 6.4.0-M2 milestone Jul 18, 2024
@nikomiranda
Copy link

Will this be released as well to spring security 5.8.x branch? I'm facing the same issue with 5.8.14

@jzheaux jzheaux moved this to Done in Spring Security Team Aug 27, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 16, 2024
@kortega90
Copy link

Hey everyone,

Just wanted to share that the annoying AnnotationConfigurationException related to @PreAuthorize annotations in Spring Security has finally been fixed!

The Problem:

Previously, Spring Security had trouble with @PreAuthorize annotations when used in class hierarchies. If both a parent class and a child class had the annotation, even with different expressions, it would throw an AnnotationConfigurationException because it incorrectly detected them as duplicates.

The Solution:

This issue has been resolved in Spring Security version 6.4.0-M2. The framework now correctly handles @PreAuthorize inheritance, allowing you to define different security expressions at different levels of your class hierarchy without conflicts.

How to Update:

To get this fix, simply update your Spring Security dependency to version 6.4.0-M2 or higher:

org.springframework.security spring-security-core 6.4.0-M2 emember to update all your Spring Security dependencies to the same version for consistency.

Important Note:

Keep in mind that 6.4.0-M2 is a pre-release version (milestone). While it includes this important fix, it might contain other bugs or instabilities. If you're looking for a more stable release, you might consider waiting for the official 6.4.0 release.

Hope this helps! Let me know if you have any questions.

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: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
Status: Done
Development

No branches or pull requests

7 participants