diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DotNames.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DotNames.java index 620ab6a018e14..accb9a4c988ba 100644 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DotNames.java +++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DotNames.java @@ -1,5 +1,7 @@ package io.quarkus.security.deployment; +import java.lang.annotation.Inherited; + import jakarta.annotation.security.DenyAll; import jakarta.annotation.security.PermitAll; import jakarta.annotation.security.RolesAllowed; @@ -17,6 +19,9 @@ public final class DotNames { public static final DotName DENY_ALL = DotName.createSimple(DenyAll.class.getName()); public static final DotName PERMIT_ALL = DotName.createSimple(PermitAll.class.getName()); + // used to make the above annotations appear as @Inherited to Arc + public static final DotName INHERITED = DotName.createSimple(Inherited.class.getName()); + private DotNames() { } } diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java index 594294fe9f05d..5c91e16e5f099 100644 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java +++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java @@ -4,6 +4,7 @@ import static io.quarkus.gizmo.MethodDescriptor.ofMethod; import static io.quarkus.security.deployment.DotNames.AUTHENTICATED; import static io.quarkus.security.deployment.DotNames.DENY_ALL; +import static io.quarkus.security.deployment.DotNames.INHERITED; import static io.quarkus.security.deployment.DotNames.PERMISSIONS_ALLOWED; import static io.quarkus.security.deployment.DotNames.PERMIT_ALL; import static io.quarkus.security.deployment.DotNames.ROLES_ALLOWED; @@ -559,6 +560,17 @@ void transformSecurityAnnotations(BuildProducer } } + /* + * Transform all security annotations to be {@code @Inherited} + */ + @BuildStep + void makeSecurityAnnotationsInherited(BuildProducer transformer) { + Set securityAnnotationNames = Set.of(PERMIT_ALL, DENY_ALL, AUTHENTICATED, PERMISSIONS_ALLOWED, ROLES_ALLOWED); + transformer.produce(new AnnotationsTransformerBuildItem(AnnotationTransformation.forClasses() + .whenClass(c -> securityAnnotationNames.contains(c.name())) + .transform(c -> c.add(AnnotationInstance.builder(INHERITED).build())))); + } + @BuildStep PermissionsAllowedMetaAnnotationBuildItem transformPermissionsAllowedMetaAnnotations( BeanArchiveIndexBuildItem beanArchiveBuildItem, diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/CdiClassLevelInheritanceTest.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/CdiClassLevelInheritanceTest.java new file mode 100644 index 0000000000000..fb84bc6487c83 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/CdiClassLevelInheritanceTest.java @@ -0,0 +1,93 @@ +package io.quarkus.security.test.cdi; + +import static io.quarkus.security.test.utils.IdentityMock.ADMIN; +import static io.quarkus.security.test.utils.IdentityMock.ANONYMOUS; +import static io.quarkus.security.test.utils.IdentityMock.USER; +import static io.quarkus.security.test.utils.SecurityTestUtils.assertFailureFor; +import static io.quarkus.security.test.utils.SecurityTestUtils.assertSuccess; + +import java.util.Collections; +import java.util.Set; + +import jakarta.inject.Inject; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.security.ForbiddenException; +import io.quarkus.security.StringPermission; +import io.quarkus.security.UnauthorizedException; +import io.quarkus.security.test.cdi.app.TestException; +import io.quarkus.security.test.cdi.inheritance.AuthenticatedBean; +import io.quarkus.security.test.cdi.inheritance.DenyAllBean; +import io.quarkus.security.test.cdi.inheritance.PermissionsAllowedBean; +import io.quarkus.security.test.cdi.inheritance.RolesAllowedBean; +import io.quarkus.security.test.cdi.inheritance.SubclassAuthenticatedBean; +import io.quarkus.security.test.cdi.inheritance.SubclassDenyAllBean; +import io.quarkus.security.test.cdi.inheritance.SubclassPermissionsAllowedBean; +import io.quarkus.security.test.cdi.inheritance.SubclassRolesAllowedBean; +import io.quarkus.security.test.utils.AuthData; +import io.quarkus.security.test.utils.IdentityMock; +import io.quarkus.security.test.utils.SecurityTestUtils; +import io.quarkus.test.QuarkusUnitTest; + +public class CdiClassLevelInheritanceTest { + + @Inject + SubclassDenyAllBean subclassDenyAll; + + @Inject + SubclassRolesAllowedBean subclassRolesAllowed; + + @Inject + SubclassAuthenticatedBean subclassAuthenticated; + + @Inject + SubclassPermissionsAllowedBean subclassPermissionsAllowed; + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(IdentityMock.class, + AuthData.class, + DenyAllBean.class, + RolesAllowedBean.class, + AuthenticatedBean.class, + PermissionsAllowedBean.class, + SubclassPermissionsAllowedBean.class, + SubclassDenyAllBean.class, + SubclassRolesAllowedBean.class, + SubclassAuthenticatedBean.class, + TestException.class, + SecurityTestUtils.class)); + + @Test + public void testDenyAllInherited() { + assertFailureFor(() -> subclassDenyAll.ping(), UnauthorizedException.class, ANONYMOUS); + assertFailureFor(() -> subclassDenyAll.ping(), ForbiddenException.class, USER, ADMIN); + } + + @Test + public void testRolesAllowedInherited() { + assertFailureFor(() -> subclassRolesAllowed.ping(), UnauthorizedException.class, ANONYMOUS); + assertFailureFor(() -> subclassRolesAllowed.ping(), ForbiddenException.class, USER); + assertSuccess(() -> subclassRolesAllowed.ping(), RolesAllowedBean.class.getSimpleName(), ADMIN); + } + + @Test + public void testAuthenticatedInherited() { + assertFailureFor(() -> subclassAuthenticated.ping(), UnauthorizedException.class, ANONYMOUS); + assertSuccess(() -> subclassAuthenticated.ping(), AuthenticatedBean.class.getSimpleName(), USER); + assertSuccess(() -> subclassAuthenticated.ping(), AuthenticatedBean.class.getSimpleName(), ADMIN); + } + + @Test + public void testPermissionAllowedInherited() { + AuthData USER_READ = new AuthData(Collections.singleton("user_read"), false, "user_read", + Set.of(new StringPermission("read"))); + AuthData USER_WRITE = new AuthData(Collections.singleton("user_write"), false, "user_write", + Set.of(new StringPermission("write"))); + assertSuccess(() -> subclassPermissionsAllowed.ping(), PermissionsAllowedBean.class.getSimpleName(), USER_READ); + assertFailureFor(() -> subclassPermissionsAllowed.ping(), ForbiddenException.class, USER_WRITE); + } +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/AuthenticatedBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/AuthenticatedBean.java new file mode 100644 index 0000000000000..8fa53514394c6 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/AuthenticatedBean.java @@ -0,0 +1,14 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +import io.quarkus.security.Authenticated; + +@ApplicationScoped +@Authenticated +public class AuthenticatedBean { + + public String ping() { + return AuthenticatedBean.class.getSimpleName(); + } +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/DenyAllBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/DenyAllBean.java new file mode 100644 index 0000000000000..6c4a9cda78d2b --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/DenyAllBean.java @@ -0,0 +1,13 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.annotation.security.DenyAll; +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +@DenyAll +public class DenyAllBean { + + public String ping() { + return DenyAllBean.class.getSimpleName(); + } +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/PermissionsAllowedBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/PermissionsAllowedBean.java new file mode 100644 index 0000000000000..7d814c61d1cab --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/PermissionsAllowedBean.java @@ -0,0 +1,14 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +import io.quarkus.security.PermissionsAllowed; + +@ApplicationScoped +@PermissionsAllowed("read") +public class PermissionsAllowedBean { + + public String ping() { + return PermissionsAllowedBean.class.getSimpleName(); + } +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/RolesAllowedBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/RolesAllowedBean.java new file mode 100644 index 0000000000000..bdcc4fb6aa8b0 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/RolesAllowedBean.java @@ -0,0 +1,13 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.annotation.security.RolesAllowed; +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +@RolesAllowed("admin") +public class RolesAllowedBean { + + public String ping() { + return RolesAllowedBean.class.getSimpleName(); + } +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassAuthenticatedBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassAuthenticatedBean.java new file mode 100644 index 0000000000000..3f812ee77914f --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassAuthenticatedBean.java @@ -0,0 +1,7 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class SubclassAuthenticatedBean extends AuthenticatedBean { +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassDenyAllBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassDenyAllBean.java new file mode 100644 index 0000000000000..0dc7d854b2ad1 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassDenyAllBean.java @@ -0,0 +1,7 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class SubclassDenyAllBean extends DenyAllBean { +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassPermissionsAllowedBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassPermissionsAllowedBean.java new file mode 100644 index 0000000000000..f145ddd3ed11a --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassPermissionsAllowedBean.java @@ -0,0 +1,7 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class SubclassPermissionsAllowedBean extends PermissionsAllowedBean { +} diff --git a/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassRolesAllowedBean.java b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassRolesAllowedBean.java new file mode 100644 index 0000000000000..5009779dfaf33 --- /dev/null +++ b/extensions/security/deployment/src/test/java/io/quarkus/security/test/cdi/inheritance/SubclassRolesAllowedBean.java @@ -0,0 +1,7 @@ +package io.quarkus.security.test.cdi.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class SubclassRolesAllowedBean extends RolesAllowedBean { +} diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java index c7434b5700df8..682aa3975cfb3 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java @@ -686,11 +686,29 @@ Collection extractQualifiers(AnnotationInstance annotation) * @return a collection of interceptor bindings or an empty collection */ public Collection extractInterceptorBindings(AnnotationInstance annotation) { + return extractInterceptorBindings(annotation, false); + } + + /** + * Behaves exactly as {@link #extractInterceptorBindings(AnnotationInstance)}, but if {@code onlyInherited == true}, + * then only {@code @Inherited} annotations are returned. This filtering does not apply to transitive + * bindings, those are always returned regardless of their {@code @Inherited} status. + */ + Collection extractInterceptorBindings(AnnotationInstance annotation, boolean onlyInherited) { Collection result = extractAnnotations(annotation, interceptorBindings, repeatingInterceptorBindingAnnotations); if (result.isEmpty()) { return result; } + if (onlyInherited) { + Set modifiedResult = new HashSet<>(); + for (AnnotationInstance ann : result) { + if (hasAnnotation(getInterceptorBinding(ann.name()), DotNames.INHERITED)) { + modifiedResult.add(ann); + } + } + result = modifiedResult; + } Set transitive = transitiveInterceptorBindings.get(annotation.name()); if (transitive != null) { result = new HashSet<>(result); diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java index 89e964f9cab17..e27ea1d084e2e 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java @@ -922,17 +922,13 @@ private void addClassLevelBindings(ClassInfo targetClass, Collection bindings, Set skip, boolean onlyInherited) { beanDeployment.getAnnotations(classInfo).stream() - .flatMap(a -> beanDeployment.extractInterceptorBindings(a).stream()) + .flatMap(a -> beanDeployment.extractInterceptorBindings(a, onlyInherited).stream()) .filter(a -> !skip.contains(a.name())) - .filter(a -> !onlyInherited - || beanDeployment.hasAnnotation(beanDeployment.getInterceptorBinding(a.name()), DotNames.INHERITED)) .forEach(bindings::add); if (classInfo.superClassType() != null && !classInfo.superClassType().name().equals(DotNames.OBJECT)) { ClassInfo superClass = getClassByName(beanDeployment.getBeanArchiveIndex(), classInfo.superName()); if (superClass != null) { - // proper interceptor binding inheritance only in strict mode, due to Quarkus expecting security - // annotations (such as `@RolesAllowed`) to be inherited, even though they are not `@Inherited` - doAddClassLevelBindings(superClass, bindings, skip, beanDeployment.strictCompatibility); + doAddClassLevelBindings(superClass, bindings, skip, true); } } } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnBeanTest.java index 6f08c6cf41389..1c85fdc13f597 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnBeanTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnBeanTest.java @@ -29,7 +29,6 @@ public class InheritedBindingOnBeanTest { @RegisterExtension public ArcTestContainer container = new ArcTestContainer.Builder() .beanClasses(MyBean.class, FooBinding.class, BarBinding.class, FooInterceptor.class, BarInterceptor.class) - .strictCompatibility(true) // correct interceptor binding inheritance .build(); @Test diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/transitive/InheritedTransitiveBindingOnBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/transitive/InheritedTransitiveBindingOnBeanTest.java new file mode 100644 index 0000000000000..1cd64f39e7d8c --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/transitive/InheritedTransitiveBindingOnBeanTest.java @@ -0,0 +1,122 @@ +package io.quarkus.arc.test.interceptors.bindings.inherited.transitive; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.Documented; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class InheritedTransitiveBindingOnBeanTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer.Builder() + .beanClasses(MyBean.class, + FooBinding.class, BarBinding.class, BazBinding.class, + FooInterceptor.class, BarInterceptor.class, BazInterceptor.class) + .build(); + + @Test + public void testInterception() { + MyBean bean = Arc.container().instance(MyBean.class).get(); + assertNotNull(bean); + bean.doSomething(); + assertTrue(FooInterceptor.intercepted); + assertFalse(BarInterceptor.intercepted); + assertTrue(BazInterceptor.intercepted); + } + + @FooBinding + @BarBinding + static class MySuperclass { + } + + @Singleton + static class MyBean extends MySuperclass { + void doSomething() { + } + } + + @BazBinding + @Target({ TYPE, METHOD }) + @Retention(RUNTIME) + @Documented + @InterceptorBinding + @Inherited + // must be `public`, otherwise the `_ComponentsProvider` would fail verification, because + // it accesses the `FooBinding.class` directly (we should perhaps change that to `String`?) + public @interface FooBinding { + } + + @Target({ TYPE, METHOD }) + @Retention(RUNTIME) + @Documented + @InterceptorBinding + // not @Inherited + @interface BarBinding { + } + + @Target({ TYPE, METHOD }) + @Retention(RUNTIME) + @Documented + @InterceptorBinding + // not @Inherited + @interface BazBinding { + } + + @FooBinding + @Interceptor + @Priority(1) + static class FooInterceptor { + static boolean intercepted = false; + + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + intercepted = true; + return ctx.proceed(); + } + } + + @BarBinding + @Interceptor + @Priority(1) + static class BarInterceptor { + static boolean intercepted = false; + + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + intercepted = true; + return ctx.proceed(); + } + } + + @BazBinding + @Interceptor + @Priority(1) + static class BazInterceptor { + static boolean intercepted = false; + + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + intercepted = true; + return ctx.proceed(); + } + } +}