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

PreAuthorizeExpressionAttributeRegistry#resolveAttribute can't find annotation on class when method is declared on superclass #13783

Closed
honhimW opened this issue Sep 7, 2023 · 4 comments · Fixed by #14516
Assignees
Labels
in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@honhimW
Copy link

honhimW commented Sep 7, 2023

Describe the bug
Using @PreAuthorize annotation on Class is not found when method is declared on superclass.

To Reproduce

Tips: I'm using in reactive;

Create a class and define methods like:

class SomeApi {
    @XXXMapping("/hello")
    public Response hello(Request request) {
         return ...;
    }
}

Create a controller:

@RestController
@PreAuthorize("hasAnyRole('xxx')")
class Controller extands(implement) SomeApi {
// nothing here
}
@honhimW honhimW added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 7, 2023
@honhimW
Copy link
Author

honhimW commented Dec 8, 2023

Well, I was wondering if there are any updates on this?
Basically, i personally solve this problem as follow

/**
 * @see org.springframework.security.authorization.method.PostAuthorizeExpressionAttributeRegistry#findPostAuthorizeAnnotation
 */
private PostAuthorize findPostAuthorizeAnnotation(Method method) {
    PostAuthorize postAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(method, PostAuthorize.class);
    // org.springframework.security.authorization.method.PostAuthorizeExpressionAttributeRegistry#resolveAttribute
    // Use `targetClass`(from calling method argument) instead of `method.getDeclaringClass()`.
    return (postAuthorize != null) ? postAuthorize
		: AuthorizationAnnotationUtils.findUniqueAnnotation(targetClass, PostAuthorize.class); 
}

@jzheaux jzheaux self-assigned this Dec 11, 2023
@jzheaux jzheaux added 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 type: bug A general bug labels Dec 11, 2023
kse-music added a commit to kse-music/spring-security that referenced this issue Jan 22, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Jan 22, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 1, 2024
jzheaux pushed a commit that referenced this issue Feb 1, 2024
@jzheaux jzheaux moved this to Done in Spring Security Team Feb 2, 2024
jzheaux pushed a commit that referenced this issue May 23, 2024
jzheaux added a commit that referenced this issue Jul 31, 2024
This feature unfortunately regresses pre-existing behavior
like that found in gh-15352. As such, this functionality
has been removed.

Closes gh-15352
jzheaux added a commit that referenced this issue Jul 31, 2024
For backward compatibility, this commit changes the annotation traversal
logic to match what is found in PrePostAnnotationSecurityMetadataSource.

This reverts gh-13783 which is a feature that unfortunately regressess
pre-existing behavior like that found in gh-15352. As such, that
functionality has been removed.

Issue gh-15352
@jzheaux jzheaux added the status: declined A suggestion or change that we don't feel we should currently apply label Sep 4, 2024
@kse-music
Copy link
Contributor

@jzheaux As you said in #15014 comment. In general, annotations declared lower in the hierarchy do not affect methods higher in the hierarchy. So now whether Spring Security still needs to support this scene?

@honhimW
Copy link
Author

honhimW commented Sep 19, 2024

@jzheaux @kse-music
Hello guys, 👋

I noticed the discussions regarding this feature, but I am a bit confused about whether the discussion is about deciding to remove this feature? Personally, I hope to keep this feature as I really need it. From my perspective, it is logical for a security annotation on a subclass to also affect to its superclass.

If the decision is to remove it, could you make this behavior configurable? There is a lot of nested code involved here, and most of it is protected. If I need to modify it in my project, I would have to override a lot of code to implement this behavior for the class declared with the method or the class where the bean is located.

@quaff
Copy link
Contributor

quaff commented Dec 17, 2024

FYI, I created #16295 to request restoring this feature, I don't think reverting is a good resolution for #15352.

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: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
Status: Done
4 participants