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

Clarify class-level interceptor binding inheritance #33666

Closed
Ladicek opened this issue May 29, 2023 · 5 comments · Fixed by #43367
Closed

Clarify class-level interceptor binding inheritance #33666

Ladicek opened this issue May 29, 2023 · 5 comments · Fixed by #43367
Assignees
Labels
area/arc Issue related to ARC (dependency injection) area/security kind/enhancement New feature or request
Milestone

Comments

@Ladicek
Copy link
Contributor

Ladicek commented May 29, 2023

Description

The Java language and the CDI specification (following Common Annotations) both define that class-level annotations are inherited only if they are @Inherited. By default, ArC inherits class-level annotations even if they are not @Inherited, which is confusing and wrong.

Unfortunately, it is also what Quarkus security expects -- because it uses annotations such as @RolesAllowed as [inheritable] interceptor bindings. In #33523, we fixed this in ArC strict mode, but by default, non-@Inherited annotations are still inherited.

Implementation ideas

One option is to fix inheritance of interceptor bindings for good and use annotation transformations to make the security annotations @Inherited.

@Ladicek Ladicek added kind/enhancement New feature or request area/arc Issue related to ARC (dependency injection) area/security labels May 29, 2023
@Ladicek
Copy link
Contributor Author

Ladicek commented May 29, 2023

CC @mkouba @manovotn @sberyozkin

@manovotn
Copy link
Contributor

manovotn commented May 29, 2023

...because it uses annotations such as @RolesAllowed as [inheritable] interceptor bindings.

Is there an actual reason why these annotations aren't @Inherited?
Or a use case where that would be undesirable?

One option is to fix inheritance of interceptor bindings for good and use annotation transformations to make the security annotations @inherited.

If we can transform those annotations directly, that would be the smoothest solution so +1

@manovotn
Copy link
Contributor

Looking back at this, we never took any action.

One option is to fix inheritance of interceptor bindings for good and use annotation transformations to make the security annotations @inherited.

@Ladicek did you mean to transform all of bindings in the app or just the security related ones?
CDI generally recommends having interceptor bindings marked as @Inherited and TBF I find it hard to imagine a scenario in which you'd need to not have it inherited.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 18, 2024

No, I meant just the security ones. They are not annotated @InterceptorBinding and I'm not sure they were meant to be interceptor bindings, hence they are not @Inherited. Yet we treat them as such.

@manovotn
Copy link
Contributor

Fair enough, I'll take a look into that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/security kind/enhancement New feature or request
Projects
None yet
2 participants