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

Provide Runtime Hints for Beans used in Pre/PostAuthorize Expressions #14790

Closed

Conversation

marcusdacoregio
Copy link
Contributor

Closes gh-14652

@marcusdacoregio marcusdacoregio added in: core An issue in spring-security-core type: enhancement A general enhancement labels Mar 21, 2024
@marcusdacoregio marcusdacoregio requested a review from jzheaux March 21, 2024 19:35
@marcusdacoregio marcusdacoregio self-assigned this Mar 21, 2024
@marcusdacoregio marcusdacoregio marked this pull request as ready for review March 21, 2024 19:35
Comment on lines +177 to +195
private static <A extends Annotation> A findDistinctAnnotation(AnnotatedElement annotatedElement,
Class<A> annotationType, Function<MergedAnnotation<A>, A> map) {
MergedAnnotations mergedAnnotations = MergedAnnotations.from(annotatedElement,
MergedAnnotations.SearchStrategy.TYPE_HIERARCHY, RepeatableContainers.none());
List<A> annotations = mergedAnnotations.stream(annotationType)
.map(MergedAnnotation::withNonMergedAttributes)
.map(map)
.distinct()
.toList();

return switch (annotations.size()) {
case 0 -> null;
case 1 -> annotations.get(0);
default -> throw new AnnotationConfigurationException("""
Please ensure there is one unique annotation of type @%s attributed to %s. \
Found %d competing annotations: %s""".formatted(annotationType.getName(), annotatedElement,
annotations.size(), annotations));
};
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzheaux This was copied from AuthorizationAnnotationUtils. My concern is that this logic can change and then we have to remember to update it on multiple places. Any idea on how we can reuse the same logic?

@marcusdacoregio
Copy link
Contributor Author

As per @jzheaux comments: it's not always clear from the bean factory which type it is that we should be proxying, like in the case of Spring Data, which publishes a factory bean (so the interface that has the annotations isn't yet known).
Another challenge ls that with these changes it will be possible and even likely that some classes won't be a bean. (Consider CrudRepositoyr#findAll which returns possibly-annotated domain objects).

With that said, we might have to adopt another strategy to find the components to register hints, maybe by doing a component scan.

@marcusdacoregio
Copy link
Contributor Author

I'll close this for now for the following reasons:

  • There are some corner cases that we are not yet sure what the approach should be, like those mentioned here.
  • The support won't be complete, for example, we will provide runtime hints only for SpEL inside @PreAuthorize and @PostAuthorize, however, there are more annotations that allows usage of SpEL and it might be surprising that those are not supported.
  • As of now, users can use @RegisterReflectionForBinding as described in https://docs.spring.io/spring-security/reference/native-image/method-security.html

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 type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide Native Hints for Beans used in Method Security Annotations
1 participant