spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.74k stars 5.86k forks source link

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

Closed arnaldop closed 2 months ago

arnaldop commented 4 months ago

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;
    }
}
jzheaux commented 4 months ago

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.

arnaldop commented 4 months ago

@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
}
kse-music commented 4 months ago

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;
    }
}
arnaldop commented 4 months ago

@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 commented 4 months ago

@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 commented 4 months ago

Meta-annotations are available prior to 6.3. Here is the doc for 6.2.4: https://docs.spring.io/spring-security/reference/6.2/servlet/authorization/method-security.html#meta-annotations

So I feel like Meta-Annotations is a false solution to the problem. As I mentioned previously, unless I am grossly misunderstanding Meta-Annotations, the @PreAuthorize annotation will still be present multiple times, meaning I still get the warning. I tested this theory and found it to be true.

So at this time, I see that my original report is indeed an issue, and that there is no workaround for it. But if I am incorrect, please advise. Thank you!

jzheaux commented 4 months ago

Related to https://github.com/spring-projects/spring-security/issues/13234, https://github.com/spring-projects/spring-security/issues/13490, and https://github.com/spring-projects/spring-security/issues/11436

anaconda875 commented 3 months ago

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

nikomiranda commented 1 month ago

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