From 7a710f0705027c13dd8715c32822556bffa026d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 5 Nov 2024 00:01:19 +0100 Subject: [PATCH] Drop deprecations from Quarkus Security & refactor bit --- ...ditionalDenyingUnannotatedTransformer.java | 29 +++++------ .../AdditionalRolesAllowedTransformer.java | 29 ++++++----- .../deployment/DenyUnannotatedPredicate.java | 19 +++++++ .../DenyingUnannotatedTransformer.java | 33 ------------- .../deployment/SecurityProcessor.java | 49 +++++++++++-------- .../runtime/AnonymousIdentityProvider.java | 8 +-- .../IdentityProviderManagerCreator.java | 2 +- .../QuarkusIdentityProviderManagerImpl.java | 41 +++++++++------- .../runtime/QuarkusSecurityIdentity.java | 2 +- .../runtime/SecurityCheckRecorder.java | 9 +--- .../AdditionalSecuredClassesBuildItem.java | 1 + 11 files changed, 111 insertions(+), 111 deletions(-) create mode 100644 extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyUnannotatedPredicate.java delete mode 100644 extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyingUnannotatedTransformer.java diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalDenyingUnannotatedTransformer.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalDenyingUnannotatedTransformer.java index c1053ee9185b9..44a8738900dd7 100644 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalDenyingUnannotatedTransformer.java +++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalDenyingUnannotatedTransformer.java @@ -1,35 +1,36 @@ package io.quarkus.security.deployment; import static io.quarkus.security.deployment.SecurityProcessor.createMethodDescription; -import static io.quarkus.security.spi.SecurityTransformerUtils.DENY_ALL; import java.util.Collection; -import java.util.HashSet; import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Predicate; -import org.jboss.jandex.AnnotationTarget; +import jakarta.annotation.security.DenyAll; + +import org.jboss.jandex.AnnotationTransformation; +import org.jboss.jandex.MethodInfo; -import io.quarkus.arc.processor.AnnotationsTransformer; import io.quarkus.security.spi.runtime.MethodDescription; -public class AdditionalDenyingUnannotatedTransformer implements AnnotationsTransformer { +final class AdditionalDenyingUnannotatedTransformer + implements Predicate, Consumer { private final Set methods; - public AdditionalDenyingUnannotatedTransformer(Collection methods) { - this.methods = new HashSet<>(methods); + AdditionalDenyingUnannotatedTransformer(Collection methods) { + this.methods = Set.copyOf(methods); } @Override - public boolean appliesTo(AnnotationTarget.Kind kind) { - return kind == AnnotationTarget.Kind.METHOD; + public void accept(AnnotationTransformation.TransformationContext ctx) { + ctx.add(DenyAll.class); } @Override - public void transform(TransformationContext context) { - MethodDescription methodDescription = createMethodDescription(context.getTarget().asMethod()); - if (methods.contains(methodDescription)) { - context.transform().add(DENY_ALL).done(); - } + public boolean test(MethodInfo methodInfo) { + MethodDescription methodDescription = createMethodDescription(methodInfo); + return methods.contains(methodDescription); } } diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalRolesAllowedTransformer.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalRolesAllowedTransformer.java index dd7c5d4ef7f00..74933c728b03c 100644 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalRolesAllowedTransformer.java +++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/AdditionalRolesAllowedTransformer.java @@ -3,41 +3,44 @@ import static io.quarkus.security.deployment.SecurityProcessor.createMethodDescription; import java.util.Collection; -import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Predicate; import jakarta.annotation.security.RolesAllowed; -import org.jboss.jandex.AnnotationTarget; +import org.jboss.jandex.AnnotationInstance; +import org.jboss.jandex.AnnotationTransformation; import org.jboss.jandex.AnnotationValue; import org.jboss.jandex.DotName; +import org.jboss.jandex.MethodInfo; -import io.quarkus.arc.processor.AnnotationsTransformer; import io.quarkus.security.spi.runtime.MethodDescription; -public class AdditionalRolesAllowedTransformer implements AnnotationsTransformer { +final class AdditionalRolesAllowedTransformer + implements Predicate, Consumer { private static final DotName ROLES_ALLOWED = DotName.createSimple(RolesAllowed.class.getName()); private final Set methods; private final AnnotationValue[] rolesAllowed; - public AdditionalRolesAllowedTransformer(Collection methods, List rolesAllowed) { - this.methods = new HashSet<>(methods); + AdditionalRolesAllowedTransformer(Collection methods, List rolesAllowed) { + this.methods = Set.copyOf(methods); this.rolesAllowed = rolesAllowed.stream().map(s -> AnnotationValue.createStringValue("", s)) .toArray(AnnotationValue[]::new); } @Override - public boolean appliesTo(AnnotationTarget.Kind kind) { - return kind == AnnotationTarget.Kind.METHOD; + public boolean test(MethodInfo methodInfo) { + MethodDescription method = createMethodDescription(methodInfo); + return methods.contains(method); } @Override - public void transform(TransformationContext context) { - MethodDescription method = createMethodDescription(context.getTarget().asMethod()); - if (methods.contains(method)) { - context.transform().add(ROLES_ALLOWED, AnnotationValue.createArrayValue("value", rolesAllowed)).done(); - } + public void accept(AnnotationTransformation.TransformationContext ctx) { + AnnotationInstance rolesAllowedInstance = AnnotationInstance.create(ROLES_ALLOWED, ctx.declaration().asMethod(), + new AnnotationValue[] { AnnotationValue.createArrayValue("value", rolesAllowed) }); + ctx.add(rolesAllowedInstance); } } diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyUnannotatedPredicate.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyUnannotatedPredicate.java new file mode 100644 index 0000000000000..0b438ccdc74a7 --- /dev/null +++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyUnannotatedPredicate.java @@ -0,0 +1,19 @@ +package io.quarkus.security.deployment; + +import java.util.List; +import java.util.function.Predicate; + +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.MethodInfo; + +import io.quarkus.security.spi.SecurityTransformerUtils; + +final class DenyUnannotatedPredicate implements Predicate { + + @Override + public boolean test(ClassInfo classInfo) { + List methods = classInfo.methods(); + return !SecurityTransformerUtils.hasSecurityAnnotation(classInfo) + && methods.stream().anyMatch(SecurityTransformerUtils::hasSecurityAnnotation); + } +} diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyingUnannotatedTransformer.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyingUnannotatedTransformer.java deleted file mode 100644 index 68621c1104179..0000000000000 --- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/DenyingUnannotatedTransformer.java +++ /dev/null @@ -1,33 +0,0 @@ -package io.quarkus.security.deployment; - -import static io.quarkus.security.spi.SecurityTransformerUtils.DENY_ALL; - -import java.util.List; - -import org.jboss.jandex.AnnotationTarget; -import org.jboss.jandex.ClassInfo; -import org.jboss.jandex.MethodInfo; - -import io.quarkus.arc.processor.AnnotationsTransformer; -import io.quarkus.security.spi.SecurityTransformerUtils; - -/** - * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com - */ -public class DenyingUnannotatedTransformer implements AnnotationsTransformer { - - @Override - public boolean appliesTo(AnnotationTarget.Kind kind) { - return kind == AnnotationTarget.Kind.CLASS; - } - - @Override - public void transform(TransformationContext transformationContext) { - ClassInfo classInfo = transformationContext.getTarget().asClass(); - List methods = classInfo.methods(); - if (!SecurityTransformerUtils.hasSecurityAnnotation(classInfo) - && methods.stream().anyMatch(SecurityTransformerUtils::hasSecurityAnnotation)) { - transformationContext.transform().add(DENY_ALL).done(); - } - } -} 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 68e775d055b65..82fc4151c7ede 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 @@ -39,6 +39,7 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import jakarta.annotation.security.DenyAll; import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.context.Dependent; import jakarta.inject.Singleton; @@ -90,8 +91,8 @@ import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem; import io.quarkus.deployment.builditem.nativeimage.RuntimeReinitializedClassBuildItem; import io.quarkus.deployment.execannotations.ExecutionModelAnnotationsAllowedBuildItem; +import io.quarkus.deployment.pkg.NativeConfig; import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem; -import io.quarkus.deployment.pkg.steps.NativeOrNativeSourcesBuild; import io.quarkus.gizmo.CatchBlockCreator; import io.quarkus.gizmo.ClassCreator; import io.quarkus.gizmo.ClassOutput; @@ -313,16 +314,18 @@ void recordBouncyCastleProviders(SecurityProviderRecorder recorder, } } - @BuildStep(onlyIf = NativeOrNativeSourcesBuild.class) - NativeImageFeatureBuildItem bouncyCastleFeature( + @BuildStep + NativeImageFeatureBuildItem bouncyCastleFeature(NativeConfig nativeConfig, List bouncyCastleProviders, List bouncyCastleJsseProviders) { - Optional bouncyCastleJsseProvider = getOne(bouncyCastleJsseProviders); - Optional bouncyCastleProvider = getOne(bouncyCastleProviders); + if (nativeConfig.enabled()) { + Optional bouncyCastleJsseProvider = getOne(bouncyCastleJsseProviders); + Optional bouncyCastleProvider = getOne(bouncyCastleProviders); - if (bouncyCastleJsseProvider.isPresent() || bouncyCastleProvider.isPresent()) { - return new NativeImageFeatureBuildItem("io.quarkus.security.BouncyCastleFeature"); + if (bouncyCastleJsseProvider.isPresent() || bouncyCastleProvider.isPresent()) { + return new NativeImageFeatureBuildItem("io.quarkus.security.BouncyCastleFeature"); + } } return null; } @@ -549,7 +552,10 @@ void transformSecurityAnnotations(BuildProducer List additionalSecuredMethods, SecurityBuildTimeConfig config) { if (config.denyUnannotated()) { - transformers.produce(new AnnotationsTransformerBuildItem(new DenyingUnannotatedTransformer())); + transformers.produce(new AnnotationsTransformerBuildItem(AnnotationTransformation + .forClasses() + .whenClass(new DenyUnannotatedPredicate()) + .transform(ctx -> ctx.add(DenyAll.class)))); } if (!additionalSecuredMethods.isEmpty()) { for (AdditionalSecuredMethodsBuildItem securedMethods : additionalSecuredMethods) { @@ -558,13 +564,19 @@ void transformSecurityAnnotations(BuildProducer additionalSecured.add(createMethodDescription(additionalSecuredMethod)); } if (securedMethods.rolesAllowed.isPresent()) { - transformers.produce( - new AnnotationsTransformerBuildItem(new AdditionalRolesAllowedTransformer(additionalSecured, - securedMethods.rolesAllowed.get()))); + var additionalRolesAllowedTransformer = new AdditionalRolesAllowedTransformer(additionalSecured, + securedMethods.rolesAllowed.get()); + transformers.produce(new AnnotationsTransformerBuildItem(AnnotationTransformation + .forMethods() + .whenMethod(additionalRolesAllowedTransformer) + .transform(additionalRolesAllowedTransformer))); } else { - transformers.produce( - new AnnotationsTransformerBuildItem( - new AdditionalDenyingUnannotatedTransformer(additionalSecured))); + var additionalDenyingUnannotatedTransformer = new AdditionalDenyingUnannotatedTransformer( + additionalSecured); + transformers.produce(new AnnotationsTransformerBuildItem(AnnotationTransformation + .forMethods() + .whenMethod(additionalDenyingUnannotatedTransformer) + .transform(additionalDenyingUnannotatedTransformer))); } } } @@ -799,17 +811,12 @@ void createSecurityCheckStorage(BuildProducer syntheticB recorder.registerDefaultSecurityCheck(builder, recorder.rolesAllowed(roles.toArray(new String[0]))); } } - recorder.create(builder); - syntheticBeans.produce( SyntheticBeanBuildItem.configure(SecurityCheckStorage.class) .scope(ApplicationScoped.class) .unremovable() - .creator(creator -> { - ResultHandle ret = creator.invokeStaticMethod(MethodDescriptor.ofMethod(SecurityCheckRecorder.class, - "getStorage", SecurityCheckStorage.class)); - creator.returnValue(ret); - }).done()); + .runtimeProxy(recorder.create(builder)) + .done()); } @Consume(RuntimeConfigSetupCompleteBuildItem.class) diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/AnonymousIdentityProvider.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/AnonymousIdentityProvider.java index 4f77f844b88b6..e1e7e4e17e103 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/AnonymousIdentityProvider.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/AnonymousIdentityProvider.java @@ -15,17 +15,17 @@ public class AnonymousIdentityProvider implements IdentityProvider { - private static final Principal principal = new Principal() { + private static final Principal PRINCIPAL = new Principal() { @Override public String getName() { return ""; } }; - private static final SecurityIdentity instance = new SecurityIdentity() { + private static final SecurityIdentity INSTANCE = new SecurityIdentity() { @Override public Principal getPrincipal() { - return principal; + return PRINCIPAL; } @Override @@ -77,6 +77,6 @@ public Class getRequestType() { @Override public Uni authenticate(AnonymousAuthenticationRequest request, AuthenticationRequestContext context) { - return Uni.createFrom().item(instance); + return Uni.createFrom().item(INSTANCE); } } diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/IdentityProviderManagerCreator.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/IdentityProviderManagerCreator.java index 31f6c48a99109..73326c94d2aee 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/IdentityProviderManagerCreator.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/IdentityProviderManagerCreator.java @@ -48,7 +48,7 @@ public Executor get() { public IdentityProviderManager ipm() { boolean customAnon = false; QuarkusIdentityProviderManagerImpl.Builder builder = QuarkusIdentityProviderManagerImpl.builder(); - for (IdentityProvider i : identityProviders) { + for (var i : identityProviders) { builder.addProvider(i); if (i.getRequestType() == AnonymousAuthenticationRequest.class) { customAnon = true; diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java index b79bae40d9326..6749d79d9c230 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusIdentityProviderManagerImpl.java @@ -30,7 +30,7 @@ public class QuarkusIdentityProviderManagerImpl implements IdentityProviderManager { private static final Logger log = Logger.getLogger(QuarkusIdentityProviderManagerImpl.class); - private final Map, List> providers; + private final Map, List>> providers; private final SecurityIdentityAugmentor[] augmenters; private final AuthenticationRequestContext blockingRequestContext; @@ -56,21 +56,22 @@ public Uni runBlocking(Supplier function) { */ public Uni authenticate(AuthenticationRequest request) { try { - List providers = this.providers.get(request.getClass()); + var providers = this.providers.get(request.getClass()); if (providers == null) { return Uni.createFrom().failure(new IllegalArgumentException( "No IdentityProviders were registered to handle AuthenticationRequest " + request)); } if (providers.size() == 1) { - return handleSingleProvider(providers.get(0), request); + return handleSingleProvider(getProvider(0, request, providers), request); } - return handleProviders((List) providers, request); + return handleProviders(providers, request); } catch (Throwable t) { return Uni.createFrom().failure(t); } } - private Uni handleSingleProvider(IdentityProvider identityProvider, AuthenticationRequest request) { + private Uni handleSingleProvider(IdentityProvider identityProvider, + T request) { Uni authenticated = identityProvider.authenticate(request, blockingRequestContext) .onItem().ifNull().failWith(new Supplier() { @Override @@ -101,16 +102,16 @@ public Uni apply(SecurityIdentity securityIdentity) * @return The first identity provider that was registered with this type */ public SecurityIdentity authenticateBlocking(AuthenticationRequest request) { - List providers = this.providers.get(request.getClass()); + var providers = this.providers.get(request.getClass()); if (providers == null) { throw new IllegalArgumentException( "No IdentityProviders were registered to handle AuthenticationRequest " + request); } - return (SecurityIdentity) handleProviders((List) providers, request).await().indefinitely(); + return handleProviders(providers, request).await().indefinitely(); } - private Uni handleProviders( - List> providers, T request) { + private Uni handleProviders( + List> providers, AuthenticationRequest request) { return handleProvider(0, providers, request) .onItem() .transformToUni(new Function>() { @@ -121,15 +122,15 @@ public Uni apply(SecurityIdentity securityIdentity) }); } - private Uni handleProvider(int pos, List> providers, - T request) { + private Uni handleProvider(int pos, List> providers, + AuthenticationRequest request) { if (pos == providers.size()) { //we failed to authentication log.debug("Authentication failed as providers would authenticate the request"); return Uni.createFrom().failure(new AuthenticationFailedException()); } - IdentityProvider current = providers.get(pos); - return current.authenticate(request, blockingRequestContext) + return getProvider(pos, request, providers) + .authenticate(request, blockingRequestContext) .onItem().transformToUni(new Function>() { @Override public Uni apply(SecurityIdentity securityIdentity) { @@ -156,6 +157,12 @@ public Uni apply(SecurityIdentity securityIdentity) { }); } + @SuppressWarnings("unchecked") + private static IdentityProvider getProvider(int pos, T ignored, + List> providers) { + return (IdentityProvider) providers.get(pos); + } + /** * Creates a builder for constructing instances of {@link QuarkusIdentityProviderManagerImpl} * @@ -173,7 +180,7 @@ public static class Builder { Builder() { } - private final Map, List> providers = new HashMap<>(); + private final Map, List>> providers = new HashMap<>(); private final List augmentors = new ArrayList<>(); private BlockingSecurityExecutor blockingExecutor; private boolean built = false; @@ -184,7 +191,7 @@ public static class Builder { * @param provider The provider * @return this builder */ - public Builder addProvider(IdentityProvider provider) { + public Builder addProvider(IdentityProvider provider) { if (built) { throw new IllegalStateException("manager has already been built"); } @@ -230,8 +237,8 @@ public QuarkusIdentityProviderManagerImpl build() { throw new IllegalStateException( "No AnonymousIdentityProvider registered. An instance of AnonymousIdentityProvider must be provided to allow the Anonymous identity to be created."); } - for (List providers : providers.values()) { - providers.sort(new Comparator() { + for (List> providers : providers.values()) { + providers.sort(new Comparator>() { @Override public int compare(IdentityProvider o1, IdentityProvider o2) { return Integer.compare(o2.priority(), o1.priority()); diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityIdentity.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityIdentity.java index db269ff6ff9dc..b99d800fcef6e 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityIdentity.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/QuarkusSecurityIdentity.java @@ -96,7 +96,7 @@ public Uni checkPermission(Permission permission) { if (results.size() == 1) { return results.get(0); } - return Uni.combine().all().unis(results).combinedWith(new Function, Boolean>() { + return Uni.combine().all().unis(results).with(new Function, Boolean>() { @Override public Boolean apply(List o) { Boolean result = null; diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java index b68221ad2c0c7..6667ba40b89ca 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java @@ -50,14 +50,9 @@ public class SecurityCheckRecorder { private static final Logger LOGGER = Logger.getLogger(SecurityCheckRecorder.class); - private static volatile SecurityCheckStorage storage; private static final Set configExpRolesAllowedChecks = ConcurrentHashMap.newKeySet(); private static volatile boolean runtimeConfigReady = false; - public static SecurityCheckStorage getStorage() { - return storage; - } - public SecurityCheck denyAll() { return DenyAllCheck.INSTANCE; } @@ -358,8 +353,8 @@ public void addMethod(RuntimeValue builder, String builder.getValue().registerCheck(className, methodName, parameterTypes, securityCheck); } - public void create(RuntimeValue builder) { - storage = builder.getValue().create(); + public SecurityCheckStorage create(RuntimeValue builder) { + return builder.getValue().create(); } public void resolveRolesAllowedConfigExpRoles() { diff --git a/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredClassesBuildItem.java b/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredClassesBuildItem.java index 6da09628ada14..eec1ddd44f649 100644 --- a/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredClassesBuildItem.java +++ b/extensions/security/spi/src/main/java/io/quarkus/security/spi/AdditionalSecuredClassesBuildItem.java @@ -14,6 +14,7 @@ * * @deprecated use {@link AdditionalSecuredMethodsBuildItem} */ +@Deprecated(forRemoval = true, since = "2.15") public final class AdditionalSecuredClassesBuildItem extends MultiBuildItem { public final Collection additionalSecuredClasses;