From 712eb128fb83b5f0cb2a98318b39fa7604b88d5e Mon Sep 17 00:00:00 2001 From: George Gastaldi Date: Tue, 23 Jan 2024 20:36:12 -0300 Subject: [PATCH 01/15] Register JDBC RowSet required bundle (cherry picked from commit 966126615030c26567e31253ce509886fd755004) --- .../java/io/quarkus/agroal/deployment/AgroalProcessor.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java b/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java index 740b255164904..7d5446dd7378a 100644 --- a/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java +++ b/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java @@ -58,6 +58,7 @@ import io.quarkus.deployment.builditem.RemovedResourceBuildItem; import io.quarkus.deployment.builditem.SslNativeConfigBuildItem; import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBuildItem; +import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBundleBuildItem; import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem; import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem; import io.quarkus.maven.dependency.ArtifactKey; @@ -408,4 +409,9 @@ void adaptOpenTelemetryJdbcInstrumentationForNative(BuildProducer Date: Wed, 24 Jan 2024 15:03:16 +0200 Subject: [PATCH 02/15] Ensure that response body of unsuccessful SSE request can be read Fixes: #38325 (cherry picked from commit e2668c5892a4c7e50e83f971a36047601f4526d2) --- .../reactive/jackson/test/MultiSseTest.java | 48 ++++++++++++++++++- .../handlers/ClientSendRequestHandler.java | 4 +- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/extensions/resteasy-reactive/rest-client-reactive-jackson/deployment/src/test/java/io/quarkus/rest/client/reactive/jackson/test/MultiSseTest.java b/extensions/resteasy-reactive/rest-client-reactive-jackson/deployment/src/test/java/io/quarkus/rest/client/reactive/jackson/test/MultiSseTest.java index 629b881a93bec..6e8774f400e36 100644 --- a/extensions/resteasy-reactive/rest-client-reactive-jackson/deployment/src/test/java/io/quarkus/rest/client/reactive/jackson/test/MultiSseTest.java +++ b/extensions/resteasy-reactive/rest-client-reactive-jackson/deployment/src/test/java/io/quarkus/rest/client/reactive/jackson/test/MultiSseTest.java @@ -8,20 +8,26 @@ import java.util.Objects; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Predicate; +import jakarta.ws.rs.DefaultValue; import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.Produces; +import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; import jakarta.ws.rs.sse.OutboundSseEvent; import jakarta.ws.rs.sse.Sse; import jakarta.ws.rs.sse.SseEventSink; +import org.eclipse.microprofile.rest.client.annotation.ClientHeaderParam; import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; +import org.jboss.resteasy.reactive.RestHeader; import org.jboss.resteasy.reactive.RestStreamElementType; import org.jboss.resteasy.reactive.client.SseEvent; import org.jboss.resteasy.reactive.client.SseEventFilter; @@ -31,6 +37,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; +import io.quarkus.rest.client.reactive.ClientExceptionMapper; import io.quarkus.rest.client.reactive.QuarkusRestClientBuilder; import io.quarkus.test.QuarkusUnitTest; import io.quarkus.test.common.http.TestHTTPResource; @@ -53,6 +60,29 @@ void shouldConsume() { await().atMost(5, TimeUnit.SECONDS) .untilAsserted( () -> assertThat(resultList).containsExactly("foo", "bar")); + + } + + @Test + void shouldReadBodyFromFailedResponse() { + var errorBody = new AtomicReference(); + createClient() + .fail() + .subscribe().with(new Consumer() { + @Override + public void accept(Object o) { + + } + }, new Consumer<>() { + @Override + public void accept(Throwable t) { + errorBody.set(t.getMessage()); + } + }); + + await().atMost(5, TimeUnit.SECONDS) + .untilAsserted( + () -> assertThat(errorBody.get()).isEqualTo("invalid input provided")); } @Test @@ -209,6 +239,11 @@ public interface SseClient { @Produces(MediaType.SERVER_SENT_EVENTS) Multi get(); + @GET + @Produces(MediaType.SERVER_SENT_EVENTS) + @ClientHeaderParam(name = "fail", value = "true") + Multi fail(); + @GET @Path("/json") @Produces(MediaType.SERVER_SENT_EVENTS) @@ -239,6 +274,14 @@ public interface SseClient { @Produces(MediaType.SERVER_SENT_EVENTS) @SseEventFilter(CustomFilter.class) Multi> eventWithFilter(); + + @ClientExceptionMapper + static RuntimeException toException(Response response) { + if (response.getStatusInfo().getStatusCode() == 400) { + return new IllegalArgumentException(response.readEntity(String.class)); + } + return null; + } } public static class CustomFilter implements Predicate> { @@ -260,7 +303,10 @@ public static class SseResource { @GET @Produces(MediaType.SERVER_SENT_EVENTS) - public Multi get() { + public Multi get(@DefaultValue("false") @RestHeader boolean fail) { + if (fail) { + throw new WebApplicationException(Response.status(400).entity("invalid input provided").build()); + } return Multi.createFrom().items("foo", "bar"); } diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java index 06f916d109038..06e2ba8da2477 100644 --- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java +++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java @@ -19,6 +19,7 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.MultivaluedMap; +import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Variant; import org.jboss.logging.Logger; @@ -246,7 +247,8 @@ public void handle(Void event) { requestContext.resume(); } }); - } else if (!requestContext.isRegisterBodyHandler()) { + } else if (!requestContext.isRegisterBodyHandler() + && (Response.Status.Family.familyOf(status) == Response.Status.Family.SUCCESSFUL)) { // we force the registration of a body handler if there was an error, so we can ensure the body can be read clientResponse.pause(); if (loggingScope != LoggingScope.NONE) { clientLogger.logResponse(clientResponse, false); From 0da36845806fde7ed54971ee7fec599ebbb4990b Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Wed, 24 Jan 2024 11:58:54 +0100 Subject: [PATCH 03/15] Bump Hibernate Reactive from 2.2.1.Final to 2.2.2.Final (cherry picked from commit c6f8a67ce9789aaf2ac2af2eaba188508c0a5662) --- bom/application/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bom/application/pom.xml b/bom/application/pom.xml index 0f12a2f243a89..1ee4b1581c351 100644 --- a/bom/application/pom.xml +++ b/bom/application/pom.xml @@ -103,7 +103,7 @@ 6.4.2.Final 1.14.7 6.0.6.Final - 2.2.1.Final + 2.2.2.Final 8.0.1.Final 7.0.0.Final From b192701601b58e5e237bc625f55de46328be74cb Mon Sep 17 00:00:00 2001 From: George Gastaldi Date: Wed, 24 Jan 2024 10:20:58 -0300 Subject: [PATCH 04/15] Include RowSet properties file in native image This fixes the `Resource javax/sql/rowset/rowset.properties not found` error (cherry picked from commit 131a1223de230f16efb45b714c66e140a146a0da) --- .../io/quarkus/agroal/deployment/AgroalProcessor.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java b/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java index 7d5446dd7378a..dd17a2b487338 100644 --- a/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java +++ b/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java @@ -411,7 +411,14 @@ void adaptOpenTelemetryJdbcInstrumentationForNative(BuildProducer resourceBundleProducer, + BuildProducer nativeResourceProducer, + BuildProducer reflectiveClassProducer) { + resourceBundleProducer.produce(new NativeImageResourceBundleBuildItem("com.sun.rowset.RowSetResourceBundle")); + nativeResourceProducer.produce(new NativeImageResourceBuildItem("javax/sql/rowset/rowset.properties")); + reflectiveClassProducer.produce(ReflectiveClassBuildItem.builder( + "com.sun.rowset.providers.RIOptimisticProvider", + "com.sun.rowset.providers.RIXMLProvider").build()); } } From 59758c58875aa36b3876f45b290c104ebe17e4b4 Mon Sep 17 00:00:00 2001 From: Alexey Loubyansky Date: Wed, 24 Jan 2024 09:36:39 +0100 Subject: [PATCH 05/15] Make sure extension metadata properties are not including timestamps (cherry picked from commit e14daa1f267d8d578eb5f5bfd8fe907e16b826ca) --- .../tracker/ConfigTrackingWriter.java | 105 +---------- core/processor/pom.xml | 10 + .../ExtensionAnnotationProcessor.java | 27 +-- .../quarkus/bootstrap/util/PropertyUtils.java | 172 ++++++++++++++++++ .../maven/ExtensionDescriptorMojo.java | 6 +- 5 files changed, 200 insertions(+), 120 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/configuration/tracker/ConfigTrackingWriter.java b/core/deployment/src/main/java/io/quarkus/deployment/configuration/tracker/ConfigTrackingWriter.java index f535eb9411946..b68be67a9f5bf 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/configuration/tracker/ConfigTrackingWriter.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/configuration/tracker/ConfigTrackingWriter.java @@ -12,6 +12,7 @@ import java.util.Map; import java.util.regex.Pattern; +import io.quarkus.bootstrap.util.PropertyUtils; import io.quarkus.deployment.configuration.BuildTimeConfigurationReader; import io.quarkus.runtime.LaunchMode; @@ -107,108 +108,6 @@ public static void write(Map readOptions, ConfigTrackingConfig c * @throws IOException in case of a failure */ public static void write(Writer writer, String name, String value) throws IOException { - if (value != null) { - name = toWritableValue(name, true, true); - value = toWritableValue(value, false, true); - writer.write(name); - writer.write("="); - writer.write(value); - writer.write(System.lineSeparator()); - } - } - - /* - * Converts unicodes to encoded \uxxxx and escapes - * special characters with a preceding slash - */ - - /** - * Escapes characters that are expected to be escaped when {@link java.util.Properties} load - * files from disk. - * - * @param str property name or value - * @param escapeSpace whether to escape a whitespace (should be true for property names) - * @param escapeUnicode whether to converts unicodes to encoded \uxxxx - * @return property name or value that can be written to a file - */ - private static String toWritableValue(String str, boolean escapeSpace, boolean escapeUnicode) { - int len = str.length(); - int bufLen = len * 2; - if (bufLen < 0) { - bufLen = Integer.MAX_VALUE; - } - StringBuilder outBuffer = new StringBuilder(bufLen); - - for (int x = 0; x < len; x++) { - char aChar = str.charAt(x); - // Handle common case first, selecting largest block that - // avoids the specials below - if ((aChar > 61) && (aChar < 127)) { - if (aChar == '\\') { - outBuffer.append('\\'); - outBuffer.append('\\'); - continue; - } - outBuffer.append(aChar); - continue; - } - switch (aChar) { - case ' ': - if (x == 0 || escapeSpace) { - outBuffer.append('\\'); - } - outBuffer.append(' '); - break; - case '\t': - outBuffer.append('\\'); - outBuffer.append('t'); - break; - case '\n': - outBuffer.append('\\'); - outBuffer.append('n'); - break; - case '\r': - outBuffer.append('\\'); - outBuffer.append('r'); - break; - case '\f': - outBuffer.append('\\'); - outBuffer.append('f'); - break; - case '=': // Fall through - case ':': // Fall through - case '#': // Fall through - case '!': - outBuffer.append('\\'); - outBuffer.append(aChar); - break; - default: - if (((aChar < 0x0020) || (aChar > 0x007e)) & escapeUnicode) { - outBuffer.append('\\'); - outBuffer.append('u'); - outBuffer.append(toHex((aChar >> 12) & 0xF)); - outBuffer.append(toHex((aChar >> 8) & 0xF)); - outBuffer.append(toHex((aChar >> 4) & 0xF)); - outBuffer.append(toHex(aChar & 0xF)); - } else { - outBuffer.append(aChar); - } - } - } - return outBuffer.toString(); - } - - /** - * Convert a nibble to a hex character - * - * @param nibble the nibble to convert. - */ - private static char toHex(int nibble) { - return hexDigit[(nibble & 0xF)]; + PropertyUtils.store(writer, name, value); } - - /** A table of hex digits */ - private static final char[] hexDigit = { - '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' - }; } diff --git a/core/processor/pom.xml b/core/processor/pom.xml index 924a0ec371883..8427a552eb841 100644 --- a/core/processor/pom.xml +++ b/core/processor/pom.xml @@ -32,6 +32,16 @@ com.fasterxml.jackson.core jackson-databind + + io.quarkus + quarkus-bootstrap-app-model + + + * + * + + + diff --git a/core/processor/src/main/java/io/quarkus/annotation/processor/ExtensionAnnotationProcessor.java b/core/processor/src/main/java/io/quarkus/annotation/processor/ExtensionAnnotationProcessor.java index 665fa57ad594c..3ac39611cfff3 100644 --- a/core/processor/src/main/java/io/quarkus/annotation/processor/ExtensionAnnotationProcessor.java +++ b/core/processor/src/main/java/io/quarkus/annotation/processor/ExtensionAnnotationProcessor.java @@ -76,6 +76,7 @@ import io.quarkus.annotation.processor.generate_doc.ConfigDocItemScanner; import io.quarkus.annotation.processor.generate_doc.ConfigDocWriter; import io.quarkus.annotation.processor.generate_doc.DocGeneratorUtil; +import io.quarkus.bootstrap.util.PropertyUtils; public class ExtensionAnnotationProcessor extends AbstractProcessor { @@ -239,23 +240,23 @@ public FileVisitResult postVisitDirectory(final Path dir, final IOException exc) } } - try { - - final FileObject listResource = filer.createResource(StandardLocation.CLASS_OUTPUT, "", - "META-INF/quarkus-javadoc.properties"); - try (OutputStream os = listResource.openOutputStream()) { - try (BufferedOutputStream bos = new BufferedOutputStream(os)) { - try (OutputStreamWriter osw = new OutputStreamWriter(bos, StandardCharsets.UTF_8)) { - try (BufferedWriter bw = new BufferedWriter(osw)) { - javaDocProperties.store(bw, Constants.EMPTY); + if (!javaDocProperties.isEmpty()) { + try { + final FileObject listResource = filer.createResource(StandardLocation.CLASS_OUTPUT, "", + "META-INF/quarkus-javadoc.properties"); + try (OutputStream os = listResource.openOutputStream()) { + try (BufferedOutputStream bos = new BufferedOutputStream(os)) { + try (OutputStreamWriter osw = new OutputStreamWriter(bos, StandardCharsets.UTF_8)) { + try (BufferedWriter bw = new BufferedWriter(osw)) { + PropertyUtils.store(javaDocProperties, bw); + } } } } + } catch (IOException e) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, "Failed to write javadoc properties: " + e); + return; } - - } catch (IOException e) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, "Failed to write javadoc properties: " + e); - return; } try { diff --git a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/bootstrap/util/PropertyUtils.java b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/bootstrap/util/PropertyUtils.java index ae163f7d9581e..5a15d4b205b38 100644 --- a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/bootstrap/util/PropertyUtils.java +++ b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/bootstrap/util/PropertyUtils.java @@ -1,8 +1,18 @@ package io.quarkus.bootstrap.util; +import java.io.BufferedWriter; +import java.io.IOException; +import java.io.Writer; +import java.nio.file.Files; +import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.Properties; /** * @@ -67,4 +77,166 @@ public static final boolean getBoolean(String name, boolean notFoundValue) { final String value = getProperty(name, (notFoundValue ? TRUE : FALSE)); return value.isEmpty() ? true : Boolean.parseBoolean(value); } + + /** + * Stores properties into a file sorting the keys alphabetically and following + * {@link Properties#store(Writer, String)} format but skipping the timestamp and comments. + * + * @param properties properties to store + * @param file target file + * @throws IOException in case of a failure + */ + public static void store(Properties properties, Path file) throws IOException { + try (BufferedWriter writer = Files.newBufferedWriter(file)) { + store(properties, writer); + } + } + + /** + * Stores properties into a file sorting the keys alphabetically and following + * {@link Properties#store(Writer, String)} format but skipping the timestamp and comments. + * + * @param properties properties to store + * @param writer target writer + * @throws IOException in case of a failure + */ + public static void store(Properties properties, Writer writer) throws IOException { + final List names = new ArrayList<>(properties.size()); + for (var name : properties.keySet()) { + names.add(name == null ? null : name.toString()); + } + Collections.sort(names); + for (String name : names) { + store(writer, name, properties.getProperty(name)); + } + } + + /** + * Stores a map of strings into a file sorting the keys alphabetically and following + * {@link Properties#store(Writer, String)} format but skipping the timestamp and comments. + * + * @param properties properties to store + * @param file target file + * @throws IOException in case of a failure + */ + public static void store(Map properties, Path file) throws IOException { + final List names = new ArrayList<>(properties.keySet()); + Collections.sort(names); + try (BufferedWriter writer = Files.newBufferedWriter(file)) { + for (String name : names) { + store(writer, name, properties.get(name)); + } + } + } + + /** + * Writes a config option with its value to the target writer, + * possibly applying some transformations, such as character escaping + * prior to writing. + * + * @param writer target writer + * @param name option name + * @param value option value + * @throws IOException in case of a failure + */ + public static void store(Writer writer, String name, String value) throws IOException { + if (value != null) { + name = toWritableValue(name, true, true); + value = toWritableValue(value, false, true); + writer.write(name); + writer.write("="); + writer.write(value); + writer.write(System.lineSeparator()); + } + } + + /** + * Escapes characters that are expected to be escaped when {@link java.util.Properties} load + * files from disk. + * + * @param str property name or value + * @param escapeSpace whether to escape a whitespace (should be true for property names) + * @param escapeUnicode whether to converts unicodes to encoded \uxxxx + * @return property name or value that can be written to a file + */ + private static String toWritableValue(String str, boolean escapeSpace, boolean escapeUnicode) { + int len = str.length(); + int bufLen = len * 2; + if (bufLen < 0) { + bufLen = Integer.MAX_VALUE; + } + StringBuilder outBuffer = new StringBuilder(bufLen); + + for (int x = 0; x < len; x++) { + char aChar = str.charAt(x); + // Handle common case first, selecting largest block that + // avoids the specials below + if ((aChar > 61) && (aChar < 127)) { + if (aChar == '\\') { + outBuffer.append('\\'); + outBuffer.append('\\'); + continue; + } + outBuffer.append(aChar); + continue; + } + switch (aChar) { + case ' ': + if (x == 0 || escapeSpace) { + outBuffer.append('\\'); + } + outBuffer.append(' '); + break; + case '\t': + outBuffer.append('\\'); + outBuffer.append('t'); + break; + case '\n': + outBuffer.append('\\'); + outBuffer.append('n'); + break; + case '\r': + outBuffer.append('\\'); + outBuffer.append('r'); + break; + case '\f': + outBuffer.append('\\'); + outBuffer.append('f'); + break; + case '=': // Fall through + case ':': // Fall through + case '#': // Fall through + case '!': + outBuffer.append('\\'); + outBuffer.append(aChar); + break; + default: + if (((aChar < 0x0020) || (aChar > 0x007e)) & escapeUnicode) { + outBuffer.append('\\'); + outBuffer.append('u'); + outBuffer.append(toHex((aChar >> 12) & 0xF)); + outBuffer.append(toHex((aChar >> 8) & 0xF)); + outBuffer.append(toHex((aChar >> 4) & 0xF)); + outBuffer.append(toHex(aChar & 0xF)); + } else { + outBuffer.append(aChar); + } + } + } + return outBuffer.toString(); + } + + /** + * Convert a nibble to a hex character + * + * @param nibble the nibble to convert. + */ + private static char toHex(int nibble) { + return hexDigit[(nibble & 0xF)]; + } + + /** A table of hex digits */ + private static final char[] hexDigit = { + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' + }; } diff --git a/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java b/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java index bcde8aacfae7e..0ae05193c4cb1 100644 --- a/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java +++ b/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java @@ -69,6 +69,7 @@ import io.quarkus.bootstrap.resolver.maven.workspace.LocalProject; import io.quarkus.bootstrap.resolver.maven.workspace.LocalWorkspace; import io.quarkus.bootstrap.util.DependencyUtils; +import io.quarkus.bootstrap.util.PropertyUtils; import io.quarkus.devtools.project.extensions.ScmInfoProvider; import io.quarkus.fs.util.ZipUtils; import io.quarkus.maven.capabilities.CapabilitiesConfig; @@ -388,10 +389,7 @@ public void execute() throws MojoExecutionException { final Path output = outputDirectory.toPath().resolve(BootstrapConstants.META_INF); try { Files.createDirectories(output); - try (BufferedWriter writer = Files - .newBufferedWriter(output.resolve(BootstrapConstants.DESCRIPTOR_FILE_NAME))) { - props.store(writer, "Generated by extension-descriptor"); - } + PropertyUtils.store(props, output.resolve(BootstrapConstants.DESCRIPTOR_FILE_NAME)); } catch (IOException e) { throw new MojoExecutionException( "Failed to persist extension descriptor " + output.resolve(BootstrapConstants.DESCRIPTOR_FILE_NAME), From e38d464ad3000022e06148eb8e3003f4a5e78808 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Wed, 24 Jan 2024 18:30:30 +0000 Subject: [PATCH 06/15] Make it easier to get the default OIDC metadata (cherry picked from commit 658321df5f87f51f519847b1cd4c708eeaaa5e79) --- .../java/io/quarkus/oidc/runtime/TenantConfigContext.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java index 556029fcbb48c..82d9091402c77 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java @@ -9,6 +9,7 @@ import org.jboss.logging.Logger; import io.quarkus.oidc.OIDCException; +import io.quarkus.oidc.OidcConfigurationMetadata; import io.quarkus.oidc.OidcTenantConfig; import io.quarkus.oidc.common.runtime.OidcCommonUtils; import io.quarkus.runtime.configuration.ConfigurationException; @@ -158,6 +159,10 @@ public OidcTenantConfig getOidcTenantConfig() { return oidcConfig; } + public OidcConfigurationMetadata getOidcMetadata() { + return provider != null ? provider.getMetadata() : null; + } + public SecretKey getStateEncryptionKey() { return stateSecretKey; } From f0c20be48454de5369340a8700b9acaaaffd335f Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Wed, 24 Jan 2024 16:55:07 +0100 Subject: [PATCH 07/15] Use UpdateDependencyVersionOperation first to update Quarkus version It is important to try that first as that recipe updates the OpenRewrite model, and thus the BOM, for the following recipes. This is important for the recipe syncing the Hibernate ORM modelgen annotation processor with the version coming from the BOM as we want to sync it with the updated version. (cherry picked from commit f2fda625c9a0cfb52bca816835aa5a79f24305b7) --- .../devtools/project/update/rewrite/QuarkusUpdates.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/update/rewrite/QuarkusUpdates.java b/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/update/rewrite/QuarkusUpdates.java index 7d6e8f3960bfb..9989ca2a42a5f 100644 --- a/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/update/rewrite/QuarkusUpdates.java +++ b/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/update/rewrite/QuarkusUpdates.java @@ -39,7 +39,12 @@ public static FetchResult createRecipe(MessageWriter log, Path target, MavenArti } switch (request.buildTool) { case MAVEN: - recipe.addOperation(new UpdatePropertyOperation("quarkus.platform.version", request.targetVersion)) + recipe.addOperation( + new UpdateDependencyVersionOperation("io.quarkus.platform", "quarkus-bom", request.targetVersion)) + .addOperation(new UpdateDependencyVersionOperation("io.quarkus", "quarkus-bom", request.targetVersion)) + .addOperation(new UpdateDependencyVersionOperation("io.quarkus", "quarkus-universe-bom", + request.targetVersion)) + .addOperation(new UpdatePropertyOperation("quarkus.platform.version", request.targetVersion)) .addOperation(new UpdatePropertyOperation("quarkus.version", request.targetVersion)) .addOperation(new UpdatePropertyOperation("quarkus-plugin.version", request.targetVersion)); if (request.kotlinVersion != null) { From edeef990bf5cd8b648a734e106ed0021832a3e04 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 25 Jan 2024 15:41:07 +0100 Subject: [PATCH 08/15] Update Gradle Maven extensions (cherry picked from commit 11856be840bc4b815e19043e174f19bdee24195c) --- .mvn/extensions.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml index 3dd605930da93..1cb948d149067 100644 --- a/.mvn/extensions.xml +++ b/.mvn/extensions.xml @@ -2,11 +2,11 @@ com.gradle gradle-enterprise-maven-extension - 1.18.1 + 1.20 com.gradle common-custom-user-data-maven-extension - 1.12.2 + 1.12.5 From 3c0e278bd401bbd7f1f9db3cd319f4ce5695fde8 Mon Sep 17 00:00:00 2001 From: Alexey Loubyansky Date: Thu, 25 Jan 2024 23:02:10 +0100 Subject: [PATCH 09/15] Don't assume module that has child modules is the parent of those modules (cherry picked from commit 09c312d2a53fdd3a198eb48816d2ea65acc9b5f6) --- .../maven/workspace/WorkspaceLoader.java | 30 ++++++++++++++----- .../test/LocalWorkspaceDiscoveryTest.java | 19 +++++++++++- .../acme-backend/acme-backend-lib/pom.xml | 18 +++++++++++ .../acme-backend/acme-backend-parent/pom.xml | 19 ++++++++++++ .../acme-backend/pom.xml | 6 ++-- 5 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-lib/pom.xml create mode 100644 independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-parent/pom.xml diff --git a/independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/WorkspaceLoader.java b/independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/WorkspaceLoader.java index f057cecb6fbbd..77cd35b4963fd 100644 --- a/independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/WorkspaceLoader.java +++ b/independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/WorkspaceLoader.java @@ -166,7 +166,7 @@ LocalProject load() throws BootstrapMavenException { private void loadModule(RawModule rawModule, List newModules) { var moduleDir = rawModule.pom.getParent(); if (moduleDir == null) { - moduleDir = Path.of("/"); + moduleDir = getFsRootDir(); } if (loadedPoms.containsKey(moduleDir)) { return; @@ -190,25 +190,39 @@ private void loadModule(RawModule rawModule, List newModules) { return; } for (var module : rawModule.model.getModules()) { - moduleQueue.add( - new RawModule(rawModule, rawModule.model.getProjectDirectory().toPath().resolve(module).resolve(POM_XML))); + queueModule(rawModule.model.getProjectDirectory().toPath().resolve(module)); } for (var profile : rawModule.model.getProfiles()) { for (var module : profile.getModules()) { - moduleQueue.add(new RawModule(rawModule, - rawModule.model.getProjectDirectory().toPath().resolve(module).resolve(POM_XML))); + queueModule(rawModule.model.getProjectDirectory().toPath().resolve(module)); } } if (rawModule.parent == null) { final Path parentPom = rawModule.getParentPom(); if (parentPom != null) { - var parent = new RawModule(parentPom); - rawModule.parent = parent; - moduleQueue.add(parent); + var parentDir = parentPom.getParent(); + if (parentDir == null) { + parentDir = getFsRootDir(); + } + if (!loadedPoms.containsKey(parentDir)) { + var parent = new RawModule(parentPom); + rawModule.parent = parent; + moduleQueue.add(parent); + } } } } + private static Path getFsRootDir() { + return Path.of("/"); + } + + private void queueModule(Path dir) { + if (!loadedPoms.containsKey(dir)) { + moduleQueue.add(new RawModule(dir.resolve(POM_XML))); + } + } + @Override public Model resolveRawModel(String groupId, String artifactId, String versionConstraint) { return loadedModules.get(new GAV(groupId, artifactId, versionConstraint)); diff --git a/independent-projects/bootstrap/maven-resolver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java b/independent-projects/bootstrap/maven-resolver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java index e1b74d3a5b6ab..1af195dd079b7 100644 --- a/independent-projects/bootstrap/maven-resolver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java +++ b/independent-projects/bootstrap/maven-resolver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java @@ -375,15 +375,32 @@ public void loadWorkspaceFromModuleDirWithParentInChildDir() throws Exception { assertParents(project, "acme-parent", "acme-dependencies"); } + @Test + public void loadWorkspaceFromModuleDirWithParentInSiblingDir() throws Exception { + final URL projectUrl = Thread.currentThread().getContextClassLoader() + .getResource("workspace-parent-is-not-root-dir/acme-backend/acme-backend-lib"); + assertNotNull(projectUrl); + final Path projectDir = Paths.get(projectUrl.toURI()); + assertTrue(Files.exists(projectDir)); + final LocalProject project = LocalProject.loadWorkspace(projectDir); + + assertEquals("acme-backend-lib", project.getArtifactId()); + assertWorkspaceWithParentInChildDir(project); + + assertParents(project, "acme-backend", "acme-backend-parent", "acme-parent", "acme-dependencies"); + } + private void assertWorkspaceWithParentInChildDir(final LocalProject project) { final LocalWorkspace workspace = project.getWorkspace(); assertNotNull(workspace.getProject("org.acme", "acme")); assertNotNull(workspace.getProject("org.acme", "acme-parent")); assertNotNull(workspace.getProject("org.acme", "acme-dependencies")); assertNotNull(workspace.getProject("org.acme", "acme-backend")); + assertNotNull(workspace.getProject("org.acme", "acme-backend-parent")); + assertNotNull(workspace.getProject("org.acme", "acme-backend-lib")); assertNotNull(workspace.getProject("org.acme", "acme-backend-rest-api")); assertNotNull(workspace.getProject("org.acme", "acme-application")); - assertEquals(6, workspace.getProjects().size()); + assertEquals(8, workspace.getProjects().size()); } @Test diff --git a/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-lib/pom.xml b/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-lib/pom.xml new file mode 100644 index 0000000000000..25a6f3a7840f9 --- /dev/null +++ b/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-lib/pom.xml @@ -0,0 +1,18 @@ + + + + 4.0.0 + + + org.acme + acme-backend + 1.0.0-SNAPSHOT + ../ + + + acme-backend-lib + + diff --git a/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-parent/pom.xml b/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-parent/pom.xml new file mode 100644 index 0000000000000..116bd703767a8 --- /dev/null +++ b/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/acme-backend-parent/pom.xml @@ -0,0 +1,19 @@ + + + + 4.0.0 + + + org.acme + acme-parent + 1.0.0-SNAPSHOT + ../../acme-parent + + + acme-backend-parent + pom + + diff --git a/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/pom.xml b/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/pom.xml index 3d0a951067cb9..af09bd7d66523 100644 --- a/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/pom.xml +++ b/independent-projects/bootstrap/maven-resolver/src/test/resources/workspace-parent-is-not-root-dir/acme-backend/pom.xml @@ -8,9 +8,9 @@ org.acme - acme-parent + acme-backend-parent 1.0.0-SNAPSHOT - ../acme-parent + acme-backend-parent/ acme-backend @@ -18,6 +18,8 @@ acme-backend-rest-api + acme-backend-parent + acme-backend-lib From 2f506ec2a0b48149a72c8ace9fb2ab663db0a88b Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Thu, 25 Jan 2024 17:56:29 +0200 Subject: [PATCH 10/15] Use simpler collection creation idioms in code example (cherry picked from commit 4f4593ea79e4c1feb0beac4f21617523f1cccd33) --- docs/src/main/asciidoc/getting-started-testing.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/main/asciidoc/getting-started-testing.adoc b/docs/src/main/asciidoc/getting-started-testing.adoc index e24923671bd91..61eba1e3bb8fb 100644 --- a/docs/src/main/asciidoc/getting-started-testing.adoc +++ b/docs/src/main/asciidoc/getting-started-testing.adoc @@ -640,14 +640,14 @@ public class Profiles { public static class SingleTag implements QuarkusTestProfile { @Override public Set tags() { - return Collections.singleton("test1"); + return List.of("test1"); } } public static class MultipleTags implements QuarkusTestProfile { @Override public Set tags() { - return new HashSet<>(Arrays.asList("test1", "test2")); + return Set.of("test1", "test2"); } } } From cb9e0692101381563614bfa0465159bf66a74345 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 25 Jan 2024 23:00:25 +0000 Subject: [PATCH 11/15] Bump io.smallrye.reactive:mutiny from 2.5.1 to 2.5.5 Bumps [io.smallrye.reactive:mutiny](https://github.com/smallrye/smallrye-mutiny) from 2.5.1 to 2.5.5. - [Release notes](https://github.com/smallrye/smallrye-mutiny/releases) - [Commits](https://github.com/smallrye/smallrye-mutiny/compare/2.5.1...2.5.5) --- updated-dependencies: - dependency-name: io.smallrye.reactive:mutiny dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] (cherry picked from commit fde099e8a8a15c7a35f54a89ead362421cf848e4) --- independent-projects/arc/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/independent-projects/arc/pom.xml b/independent-projects/arc/pom.xml index c3ccc961bf2d6..63ebe84f24e03 100644 --- a/independent-projects/arc/pom.xml +++ b/independent-projects/arc/pom.xml @@ -47,7 +47,7 @@ 1.7.0 3.1.6 3.5.3.Final - 2.5.1 + 2.5.5 1.6.Final 3.25.1 From cae2591f16865e3390806194909e1a927c566f49 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Fri, 26 Jan 2024 10:13:36 +0200 Subject: [PATCH 12/15] Make sure that @WithFormRead doesn't break body handling Fixes: #38326 (cherry picked from commit 93167f09d5aff9b82050497ade21aaad6b5fd085) --- extensions/csrf-reactive/deployment/pom.xml | 10 + .../io/quarkus/csrf/reactive/CsrfTest.java | 203 ++++++++++++++++++ .../test/resources/templates/csrfToken.html | 16 ++ .../startup/RuntimeResourceDeployment.java | 3 +- 4 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 extensions/csrf-reactive/deployment/src/test/java/io/quarkus/csrf/reactive/CsrfTest.java create mode 100644 extensions/csrf-reactive/deployment/src/test/resources/templates/csrfToken.html diff --git a/extensions/csrf-reactive/deployment/pom.xml b/extensions/csrf-reactive/deployment/pom.xml index a237a9890e57c..4e23e6777192c 100644 --- a/extensions/csrf-reactive/deployment/pom.xml +++ b/extensions/csrf-reactive/deployment/pom.xml @@ -38,6 +38,16 @@ io.quarkus quarkus-vertx-http-deployment + + io.quarkus + quarkus-junit5-internal + test + + + io.rest-assured + rest-assured + test + diff --git a/extensions/csrf-reactive/deployment/src/test/java/io/quarkus/csrf/reactive/CsrfTest.java b/extensions/csrf-reactive/deployment/src/test/java/io/quarkus/csrf/reactive/CsrfTest.java new file mode 100644 index 0000000000000..4f527f65d5e1d --- /dev/null +++ b/extensions/csrf-reactive/deployment/src/test/java/io/quarkus/csrf/reactive/CsrfTest.java @@ -0,0 +1,203 @@ +package io.quarkus.csrf.reactive; + +import static io.restassured.RestAssured.given; +import static io.restassured.RestAssured.when; + +import jakarta.inject.Inject; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.FormParam; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; + +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.qute.Template; +import io.quarkus.qute.TemplateInstance; +import io.quarkus.test.QuarkusUnitTest; +import io.restassured.RestAssured; +import io.restassured.config.EncoderConfig; +import io.restassured.config.RestAssuredConfig; +import io.restassured.http.ContentType; +import io.smallrye.mutiny.Uni; + +public class CsrfTest { + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(TestResource.class) + .addAsResource("templates/csrfToken.html")); + + private final static String COOKIE_NAME = "csrf-token"; + private final static String HEADER_NAME = "X-CSRF-TOKEN"; + + @Test + public void testForm() { + String token = when() + .get("/csrfTokenForm") + .then() + .statusCode(200) + .cookie(COOKIE_NAME) + .extract() + .cookie(COOKIE_NAME); + EncoderConfig encoderConfig = EncoderConfig.encoderConfig().encodeContentTypeAs("multipart/form-data", + ContentType.TEXT); + RestAssuredConfig restAssuredConfig = RestAssured.config().encoderConfig(encoderConfig); + + //no token + given() + .cookie(COOKIE_NAME, token) + .config(restAssuredConfig) + .formParam("name", "testName") + .contentType(ContentType.URLENC) + .when() + .post("csrfTokenForm") + .then() + .statusCode(400); + + //wrong token + given() + .cookie(COOKIE_NAME, token) + .config(restAssuredConfig) + .formParam(COOKIE_NAME, "WRONG") + .formParam("name", "testName") + .contentType(ContentType.URLENC) + .when() + .post("csrfTokenForm") + .then() + .statusCode(400); + + //valid token + given() + .cookie(COOKIE_NAME, token) + .config(restAssuredConfig) + .formParam(COOKIE_NAME, token) + .formParam("name", "testName") + .contentType(ContentType.URLENC) + .when() + .post("csrfTokenForm") + .then() + .statusCode(200) + .body(Matchers.equalTo("testName")); + } + + @Test + public void testNoBody() { + String token = when().get("/csrfTokenForm") + .then().statusCode(200).cookie(COOKIE_NAME) + .extract().cookie(COOKIE_NAME); + + // no token + given() + .cookie(COOKIE_NAME, token) + .when() + .post("csrfTokenPost") + .then() + .statusCode(400); + + //wrong token + given() + .cookie(COOKIE_NAME, token) + .header(HEADER_NAME, "WRONG") + .when() + .post("csrfTokenPost") + .then() + .statusCode(400); + + //valid token + given() + .cookie(COOKIE_NAME, token) + .header(HEADER_NAME, token) + .when() + .post("csrfTokenPost") + .then() + .statusCode(200) + .body(Matchers.equalTo("no user")); + } + + @Test + public void testWithBody() { + String token = when() + .get("/csrfTokenForm") + .then() + .statusCode(200) + .cookie(COOKIE_NAME) + .extract() + .cookie(COOKIE_NAME); + + // no token + given() + .cookie(COOKIE_NAME, token) + .body("testName") + .contentType(ContentType.TEXT) + .when() + .post("csrfTokenPostBody") + .then() + .statusCode(400); + + //wrong token + given() + .cookie(COOKIE_NAME, token) + .header(HEADER_NAME, "WRONG") + .body("testName") + .contentType(ContentType.TEXT) + .when() + .post("csrfTokenPostBody") + .then() + .statusCode(400); + + //valid token => This test fails but should work + given() + .cookie(COOKIE_NAME, token) + .header(HEADER_NAME, token) + .body("testName") + .contentType(ContentType.TEXT) + .when() + .post("csrfTokenPostBody") + .then() + .statusCode(200) + .body(Matchers.equalTo("testName")); + } + + @Path("") + public static class TestResource { + + @Inject + Template csrfToken; + + @GET + @Path("/csrfTokenForm") + @Produces(MediaType.TEXT_HTML) + public TemplateInstance getCsrfTokenForm() { + return csrfToken.instance(); + } + + @POST + @Path("/csrfTokenForm") + @Consumes(MediaType.APPLICATION_FORM_URLENCODED) + @Produces(MediaType.TEXT_PLAIN) + public Uni postCsrfTokenForm(@FormParam("name") String userName) { + return Uni.createFrom().item(userName); + } + + @POST + @Path("/csrfTokenPost") + @Produces(MediaType.TEXT_PLAIN) + public Uni postJson() { + return Uni.createFrom().item("no user"); + } + + @POST + @Path("/csrfTokenPostBody") + @Consumes(MediaType.TEXT_PLAIN) + @Produces(MediaType.TEXT_PLAIN) + public Uni postJson(String body) { + return Uni.createFrom().item(body); + } + } +} diff --git a/extensions/csrf-reactive/deployment/src/test/resources/templates/csrfToken.html b/extensions/csrf-reactive/deployment/src/test/resources/templates/csrfToken.html new file mode 100644 index 0000000000000..d9363624cbc40 --- /dev/null +++ b/extensions/csrf-reactive/deployment/src/test/resources/templates/csrfToken.html @@ -0,0 +1,16 @@ + + + + + User Name Input + + +

User Name Input

+ +
+ +

Your Name:

+

+
+ + \ No newline at end of file diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java index b8697b15158c5..455355d6a1521 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java @@ -294,7 +294,8 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz, // read the body as multipart in one go handlers.add(new FormBodyHandler(bodyParameter != null, executorSupplier, method.getFileFormNames())); checkWithFormReadRequestFilters = true; - } else if (bodyParameter != null) { + } + if (bodyParameter != null) { if (!defaultBlocking) { if (!method.isBlocking()) { // allow the body to be read by chunks From 1940eba7c2139ff9d13ec749ac35dfd43ddae4ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 23 Jan 2024 22:22:04 +0100 Subject: [PATCH 13/15] Move RESTEasy Classic RBAC security checks to JAX-RS filter (cherry picked from commit f850e7d35ba1f96b66be6686c7a9e34c7178450e) --- .../deployment/ResteasyBuiltinsProcessor.java | 20 +- .../test/security/EagerSecurityCheckTest.java | 176 ++++++++++++++++++ .../resteasy/runtime/EagerSecurityFilter.java | 117 ++++++++++-- .../StandardSecurityCheckInterceptor.java | 85 +++++++++ 4 files changed, 376 insertions(+), 22 deletions(-) create mode 100644 extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java create mode 100644 extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java diff --git a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java index df5f2f1a1ff3e..2001747fbb72b 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java @@ -7,7 +7,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.stream.Collectors; import org.jboss.jandex.ClassInfo; @@ -34,6 +33,7 @@ import io.quarkus.resteasy.runtime.JaxRsSecurityConfig; import io.quarkus.resteasy.runtime.NotFoundExceptionMapper; import io.quarkus.resteasy.runtime.SecurityContextFilter; +import io.quarkus.resteasy.runtime.StandardSecurityCheckInterceptor; import io.quarkus.resteasy.runtime.UnauthorizedExceptionMapper; import io.quarkus.resteasy.runtime.vertx.JsonArrayReader; import io.quarkus.resteasy.runtime.vertx.JsonArrayWriter; @@ -41,7 +41,6 @@ import io.quarkus.resteasy.runtime.vertx.JsonObjectWriter; import io.quarkus.resteasy.server.common.deployment.ResteasyDeploymentBuildItem; import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; -import io.quarkus.vertx.http.deployment.EagerSecurityInterceptorBuildItem; import io.quarkus.vertx.http.deployment.HttpRootPathBuildItem; import io.quarkus.vertx.http.deployment.devmode.NotFoundPageDisplayableEndpointBuildItem; import io.quarkus.vertx.http.deployment.devmode.RouteDescriptionBuildItem; @@ -91,8 +90,7 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index, */ @BuildStep void setUpSecurity(BuildProducer providers, - BuildProducer additionalBeanBuildItem, Capabilities capabilities, - Optional eagerSecurityInterceptors) { + BuildProducer additionalBeanBuildItem, Capabilities capabilities) { providers.produce(new ResteasyJaxrsProviderBuildItem(UnauthorizedExceptionMapper.class.getName())); providers.produce(new ResteasyJaxrsProviderBuildItem(ForbiddenExceptionMapper.class.getName())); providers.produce(new ResteasyJaxrsProviderBuildItem(AuthenticationFailedExceptionMapper.class.getName())); @@ -102,10 +100,16 @@ void setUpSecurity(BuildProducer providers, if (capabilities.isPresent(Capability.SECURITY)) { providers.produce(new ResteasyJaxrsProviderBuildItem(SecurityContextFilter.class.getName())); additionalBeanBuildItem.produce(AdditionalBeanBuildItem.unremovableOf(SecurityContextFilter.class)); - if (eagerSecurityInterceptors.isPresent()) { - providers.produce(new ResteasyJaxrsProviderBuildItem(EagerSecurityFilter.class.getName())); - additionalBeanBuildItem.produce(AdditionalBeanBuildItem.unremovableOf(EagerSecurityFilter.class)); - } + providers.produce(new ResteasyJaxrsProviderBuildItem(EagerSecurityFilter.class.getName())); + additionalBeanBuildItem.produce(AdditionalBeanBuildItem.unremovableOf(EagerSecurityFilter.class)); + additionalBeanBuildItem.produce( + AdditionalBeanBuildItem.unremovableOf(StandardSecurityCheckInterceptor.RolesAllowedInterceptor.class)); + additionalBeanBuildItem.produce(AdditionalBeanBuildItem + .unremovableOf(StandardSecurityCheckInterceptor.PermissionsAllowedInterceptor.class)); + additionalBeanBuildItem.produce( + AdditionalBeanBuildItem.unremovableOf(StandardSecurityCheckInterceptor.PermitAllInterceptor.class)); + additionalBeanBuildItem.produce( + AdditionalBeanBuildItem.unremovableOf(StandardSecurityCheckInterceptor.AuthenticatedInterceptor.class)); } } diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java new file mode 100644 index 0000000000000..d71665b4f1292 --- /dev/null +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/EagerSecurityCheckTest.java @@ -0,0 +1,176 @@ +package io.quarkus.resteasy.test.security; + +import jakarta.annotation.security.DenyAll; +import jakarta.annotation.security.PermitAll; +import jakarta.annotation.security.RolesAllowed; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.security.Authenticated; +import io.quarkus.security.test.utils.TestIdentityController; +import io.quarkus.security.test.utils.TestIdentityProvider; +import io.quarkus.test.QuarkusUnitTest; +import io.restassured.RestAssured; +import io.restassured.http.ContentType; +import io.restassured.response.Response; +import io.vertx.core.json.JsonObject; + +/** + * Tests that {@link io.quarkus.security.spi.runtime.SecurityCheck}s are executed by Jakarta REST filters. + */ +public class EagerSecurityCheckTest { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(TestIdentityProvider.class, TestIdentityController.class, JsonResource.class, + AbstractJsonResource.class, JsonSubResource.class)); + + @BeforeAll + public static void setupUsers() { + TestIdentityController.resetRoles() + .add("admin", "admin", "admin") + .add("user", "user", "user"); + } + + @Test + public void testAuthenticated() { + testPostJson("auth", "admin", true).then().statusCode(400); + testPostJson("auth", null, true).then().statusCode(401); + testPostJson("auth", "admin", false).then().statusCode(200); + testPostJson("auth", null, false).then().statusCode(401); + } + + @Test + public void testRolesAllowed() { + testPostJson("roles", "admin", true).then().statusCode(400); + testPostJson("roles", "user", true).then().statusCode(403); + testPostJson("roles", "admin", false).then().statusCode(200); + testPostJson("roles", "user", false).then().statusCode(403); + } + + @Test + public void testRolesAllowedOverriddenMethod() { + testPostJson("/roles-overridden", "admin", true).then().statusCode(400); + testPostJson("/roles-overridden", "user", true).then().statusCode(403); + testPostJson("/roles-overridden", "admin", false).then().statusCode(200); + testPostJson("/roles-overridden", "user", false).then().statusCode(403); + } + + @Test + public void testDenyAll() { + testPostJson("deny", "admin", true).then().statusCode(403); + testPostJson("deny", null, true).then().statusCode(401); + testPostJson("deny", "admin", false).then().statusCode(403); + testPostJson("deny", null, false).then().statusCode(401); + } + + @Test + public void testDenyAllClassLevel() { + testPostJson("/sub-resource/deny-class-level-annotation", "admin", true).then().statusCode(403); + testPostJson("/sub-resource/deny-class-level-annotation", null, true).then().statusCode(401); + testPostJson("/sub-resource/deny-class-level-annotation", "admin", false).then().statusCode(403); + testPostJson("/sub-resource/deny-class-level-annotation", null, false).then().statusCode(401); + } + + @Test + public void testPermitAll() { + testPostJson("permit", "admin", true).then().statusCode(400); + testPostJson("permit", null, true).then().statusCode(400); + testPostJson("permit", "admin", false).then().statusCode(200); + testPostJson("permit", null, false).then().statusCode(200); + } + + @Test + public void testSubResource() { + testPostJson("/sub-resource/roles", "admin", true).then().statusCode(400); + testPostJson("/sub-resource/roles", "user", true).then().statusCode(403); + testPostJson("/sub-resource/roles", "admin", false).then().statusCode(200); + testPostJson("/sub-resource/roles", "user", false).then().statusCode(403); + } + + private static Response testPostJson(String path, String username, boolean invalid) { + var req = RestAssured.given(); + if (username != null) { + req = req.auth().preemptive().basic(username, username); + } + return req + .contentType(ContentType.JSON) + .body((invalid ? "}" : "") + "{\"simple\": \"obj\"}").post(path); + } + + @Path("/") + @Produces(MediaType.APPLICATION_JSON) + @Consumes(MediaType.APPLICATION_JSON) + public static class JsonResource extends AbstractJsonResource { + + @Authenticated + @Path("/auth") + @POST + public JsonObject auth(JsonObject array) { + return array.put("test", "testval"); + } + + @RolesAllowed("admin") + @Path("/roles") + @POST + public JsonObject roles(JsonObject array) { + return array.put("test", "testval"); + } + + @PermitAll + @Path("/permit") + @POST + public JsonObject permit(JsonObject array) { + return array.put("test", "testval"); + } + + @PermitAll + @Path("/sub-resource") + public JsonSubResource subResource() { + return new JsonSubResource(); + } + + @RolesAllowed("admin") + @Override + public JsonObject rolesOverridden(JsonObject array) { + return array.put("test", "testval"); + } + } + + @DenyAll + public static class JsonSubResource { + @RolesAllowed("admin") + @Path("/roles") + @POST + public JsonObject roles(JsonObject array) { + return array.put("test", "testval"); + } + + @Path("/deny-class-level-annotation") + @POST + public JsonObject denyClassLevelAnnotation(JsonObject array) { + return array.put("test", "testval"); + } + } + + public static abstract class AbstractJsonResource { + @DenyAll + @Path("/deny") + @POST + public JsonObject deny(JsonObject array) { + return array.put("test", "testval"); + } + + @Path("/roles-overridden") + @POST + public abstract JsonObject rolesOverridden(JsonObject array); + } +} diff --git a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java index 81a138aaa974e..e05df7c8dc751 100644 --- a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java +++ b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/EagerSecurityFilter.java @@ -1,11 +1,15 @@ package io.quarkus.resteasy.runtime; +import static io.quarkus.security.spi.runtime.SecurityEventHelper.AUTHORIZATION_FAILURE; +import static io.quarkus.security.spi.runtime.SecurityEventHelper.AUTHORIZATION_SUCCESS; + import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.function.Consumer; import jakarta.annotation.Priority; +import jakarta.enterprise.event.Event; import jakarta.inject.Inject; import jakarta.ws.rs.Priorities; import jakarta.ws.rs.container.ContainerRequestContext; @@ -14,7 +18,19 @@ import jakarta.ws.rs.core.Context; import jakarta.ws.rs.ext.Provider; +import org.eclipse.microprofile.config.ConfigProvider; + +import io.quarkus.arc.Arc; +import io.quarkus.security.UnauthorizedException; +import io.quarkus.security.identity.CurrentIdentityAssociation; +import io.quarkus.security.identity.SecurityIdentity; +import io.quarkus.security.spi.runtime.AuthorizationController; +import io.quarkus.security.spi.runtime.AuthorizationFailureEvent; +import io.quarkus.security.spi.runtime.AuthorizationSuccessEvent; import io.quarkus.security.spi.runtime.MethodDescription; +import io.quarkus.security.spi.runtime.SecurityCheck; +import io.quarkus.security.spi.runtime.SecurityCheckStorage; +import io.quarkus.security.spi.runtime.SecurityEventHelper; import io.quarkus.vertx.http.runtime.security.EagerSecurityInterceptorStorage; import io.vertx.ext.web.RoutingContext; @@ -29,33 +45,106 @@ public void accept(RoutingContext routingContext) { } }; private final Map> cache = new HashMap<>(); + private final EagerSecurityInterceptorStorage interceptorStorage; + private final SecurityEventHelper eventHelper; @Context ResourceInfo resourceInfo; @Inject - EagerSecurityInterceptorStorage interceptorStorage; + RoutingContext routingContext; @Inject - RoutingContext routingContext; + SecurityCheckStorage securityCheckStorage; + + @Inject + CurrentIdentityAssociation identityAssociation; + + @Inject + AuthorizationController authorizationController; + + public EagerSecurityFilter() { + var interceptorStorageHandle = Arc.container().instance(EagerSecurityInterceptorStorage.class); + this.interceptorStorage = interceptorStorageHandle.isAvailable() ? interceptorStorageHandle.get() : null; + Event event = Arc.container().beanManager().getEvent(); + this.eventHelper = new SecurityEventHelper<>(event.select(AuthorizationSuccessEvent.class), + event.select(AuthorizationFailureEvent.class), AUTHORIZATION_SUCCESS, + AUTHORIZATION_FAILURE, Arc.container().beanManager(), + ConfigProvider.getConfig().getOptionalValue("quarkus.security.events.enabled", Boolean.class).orElse(false)); + } @Override public void filter(ContainerRequestContext requestContext) throws IOException { - var description = MethodDescription.ofMethod(resourceInfo.getResourceMethod()); - var interceptor = cache.get(description); + if (authorizationController.isAuthorizationEnabled()) { + var description = MethodDescription.ofMethod(resourceInfo.getResourceMethod()); + if (interceptorStorage != null) { + applyEagerSecurityInterceptors(description); + } + applySecurityChecks(description); + } + } + + private void applySecurityChecks(MethodDescription description) { + SecurityCheck check = securityCheckStorage.getSecurityCheck(description); + if (check != null) { + if (check.isPermitAll()) { + fireEventOnAuthZSuccess(check, null); + } else { + if (check.requiresMethodArguments()) { + if (identityAssociation.getIdentity().isAnonymous()) { + var exception = new UnauthorizedException(); + if (eventHelper.fireEventOnFailure()) { + fireEventOnAuthZFailure(exception, check); + } + throw exception; + } + // security check will be performed by CDI interceptor + return; + } + if (eventHelper.fireEventOnFailure()) { + try { + check.apply(identityAssociation.getIdentity(), description, null); + } catch (Exception e) { + fireEventOnAuthZFailure(e, check); + throw e; + } + } else { + check.apply(identityAssociation.getIdentity(), description, null); + } + fireEventOnAuthZSuccess(check, identityAssociation.getIdentity()); + } + // prevent repeated security checks + routingContext.put(EagerSecurityFilter.class.getName(), resourceInfo.getResourceMethod()); + } + } + + private void fireEventOnAuthZFailure(Exception exception, SecurityCheck check) { + eventHelper.fireFailureEvent(new AuthorizationFailureEvent( + identityAssociation.getIdentity(), exception, check.getClass().getName(), + Map.of(RoutingContext.class.getName(), routingContext))); + } - if (interceptor == NULL_SENTINEL) { - return; - } else if (interceptor != null) { - interceptor.accept(routingContext); + private void fireEventOnAuthZSuccess(SecurityCheck check, SecurityIdentity securityIdentity) { + if (eventHelper.fireEventOnSuccess()) { + eventHelper.fireSuccessEvent(new AuthorizationSuccessEvent(securityIdentity, + check.getClass().getName(), Map.of(RoutingContext.class.getName(), routingContext))); } + } - interceptor = interceptorStorage.getInterceptor(description); - if (interceptor == null) { - cache.put(description, NULL_SENTINEL); - } else { - cache.put(description, interceptor); - interceptor.accept(routingContext); + private void applyEagerSecurityInterceptors(MethodDescription description) { + var interceptor = cache.get(description); + if (interceptor != NULL_SENTINEL) { + if (interceptor != null) { + interceptor.accept(routingContext); + } else { + interceptor = interceptorStorage.getInterceptor(description); + if (interceptor == null) { + cache.put(description, NULL_SENTINEL); + } else { + cache.put(description, interceptor); + interceptor.accept(routingContext); + } + } } } } diff --git a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java new file mode 100644 index 0000000000000..036ab8fcf3b74 --- /dev/null +++ b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java @@ -0,0 +1,85 @@ +package io.quarkus.resteasy.runtime; + +import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.EXECUTED; +import static io.quarkus.security.spi.runtime.SecurityHandlerConstants.SECURITY_HANDLER; + +import java.lang.reflect.Method; + +import jakarta.annotation.Priority; +import jakarta.annotation.security.DenyAll; +import jakarta.annotation.security.PermitAll; +import jakarta.annotation.security.RolesAllowed; +import jakarta.inject.Inject; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InvocationContext; + +import io.quarkus.security.Authenticated; +import io.quarkus.security.PermissionsAllowed; +import io.quarkus.security.spi.runtime.AuthorizationController; +import io.vertx.ext.web.RoutingContext; + +/** + * Security checks for RBAC annotations on endpoints are done by the {@link EagerSecurityFilter}, this interceptor + * propagates the information to the SecurityHandler to prevent repeated checks. The {@link DenyAll} security check + * is performed just once. + */ +public abstract class StandardSecurityCheckInterceptor { + + @Inject + AuthorizationController controller; + + @Inject + RoutingContext routingContext; + + @AroundInvoke + public Object intercept(InvocationContext ic) throws Exception { + if (controller.isAuthorizationEnabled()) { + Method method = routingContext.get(EagerSecurityFilter.class.getName()); + if (method != null && method.equals(ic.getMethod())) { + ic.getContextData().put(SECURITY_HANDLER, EXECUTED); + } + } + return ic.proceed(); + } + + /** + * Prevent the SecurityHandler from performing {@link RolesAllowed} security checks + */ + @Interceptor + @RolesAllowed("") + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class RolesAllowedInterceptor extends StandardSecurityCheckInterceptor { + + } + + /** + * Prevent the SecurityHandler from performing {@link PermissionsAllowed} security checks + */ + @Interceptor + @PermissionsAllowed("") + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class PermissionsAllowedInterceptor extends StandardSecurityCheckInterceptor { + + } + + /** + * Prevent the SecurityHandler from performing {@link PermitAll} security checks + */ + @Interceptor + @PermitAll + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class PermitAllInterceptor extends StandardSecurityCheckInterceptor { + + } + + /** + * Prevent the SecurityHandler from performing {@link Authenticated} security checks + */ + @Interceptor + @Authenticated + @Priority(Interceptor.Priority.PLATFORM_BEFORE) + public static final class AuthenticatedInterceptor extends StandardSecurityCheckInterceptor { + + } +} From c026b1cf6f2e07cc50b65c824d922319248d9341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 23 Jan 2024 16:40:33 +0100 Subject: [PATCH 14/15] Fix JAX-RS default security checks for inherited / transformed endpoints (cherry picked from commit 4b559e9c6d2968cb2c37d66634e493f23fcbfead) --- .../RestPathAnnotationProcessor.java | 2 +- .../deployment/ResteasyBuiltinsProcessor.java | 66 ++++++++++++++- .../security/AbstractSecurityEventTest.java | 3 +- .../DefaultRolesAllowedJaxRsTest.java | 16 +++- .../DefaultRolesAllowedStarJaxRsTest.java | 4 +- .../test/security/DenyAllJaxRsTest.java | 16 +++- .../security/UnsecuredParentResource.java | 14 ++++ .../test/security/UnsecuredResource.java | 7 +- .../security/UnsecuredResourceInterface.java | 14 ++++ .../ResteasyReactiveCommonProcessor.java | 53 ++---------- .../security/AbstractSecurityEventTest.java | 3 +- .../DefaultRolesAllowedJaxRsTest.java | 16 +++- .../DefaultRolesAllowedStarJaxRsTest.java | 4 +- .../test/security/DenyAllJaxRsTest.java | 84 ++++++++++++++++++- .../security/UnsecuredParentResource.java | 14 ++++ .../test/security/UnsecuredResource.java | 2 +- .../security/UnsecuredResourceInterface.java | 14 ++++ .../security/EagerSecurityHandler.java | 16 +++- .../deployment/SecurityProcessor.java | 11 +++ .../spi/runtime/SecurityCheckStorage.java | 5 ++ .../runtime/SecurityCheckRecorder.java | 4 + .../SecurityCheckStorageBuilder.java | 13 +++ .../spi/DefaultSecurityCheckBuildItem.java | 28 +++++++ .../common/processor/EndpointIndexer.java | 19 ----- 24 files changed, 339 insertions(+), 89 deletions(-) create mode 100644 extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java create mode 100644 extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java create mode 100644 extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java diff --git a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java index 5959d9e09c199..94b3a0993718a 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java @@ -182,7 +182,7 @@ static Optional searchPathAnnotationOnInterfaces(CombinedInd * @param resultAcc accumulator for tail-recursion * @return Collection of all interfaces und their parents. Never null. */ - private static Collection getAllClassInterfaces( + static Collection getAllClassInterfaces( CombinedIndexBuildItem index, Collection classInfos, List resultAcc) { diff --git a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java index 2001747fbb72b..e525ee2caddfd 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java @@ -1,17 +1,21 @@ package io.quarkus.resteasy.deployment; import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT; +import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.getAllClassInterfaces; import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.isRestEndpointMethod; import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.MethodInfo; +import org.jboss.logging.Logger; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; import io.quarkus.deployment.Capabilities; @@ -50,6 +54,7 @@ public class ResteasyBuiltinsProcessor { protected static final String META_INF_RESOURCES = "META-INF/resources"; + private static final Logger LOG = Logger.getLogger(ResteasyBuiltinsProcessor.class); @BuildStep void setUpDenyAllJaxRs(CombinedIndexBuildItem index, @@ -65,10 +70,42 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index, ClassInfo classInfo = index.getIndex().getClassByName(DotName.createSimple(className)); if (classInfo == null) throw new IllegalStateException("Unable to find class info for " + className); - if (!hasSecurityAnnotation(classInfo)) { - for (MethodInfo methodInfo : classInfo.methods()) { - if (isRestEndpointMethod(index, methodInfo) && !hasSecurityAnnotation(methodInfo)) { - methods.add(methodInfo); + // add unannotated class endpoints as well as parent class unannotated endpoints + addAllUnannotatedEndpoints(index, classInfo, methods); + + // interface endpoints implemented on resources are already in, now we need to resolve default interface + // methods as there, CDI interceptors won't work, therefore neither will our additional secured methods + Collection interfaces = getAllClassInterfaces(index, List.of(classInfo), new ArrayList<>()); + if (!interfaces.isEmpty()) { + final List interfaceEndpoints = new ArrayList<>(); + for (ClassInfo anInterface : interfaces) { + addUnannotatedEndpoints(index, anInterface, interfaceEndpoints); + } + // look for implementors as implementors on resource classes are secured by CDI interceptors + if (!interfaceEndpoints.isEmpty()) { + interfaceBlock: for (MethodInfo interfaceEndpoint : interfaceEndpoints) { + if (interfaceEndpoint.isDefault()) { + for (MethodInfo endpoint : methods) { + boolean nameParamsMatch = endpoint.name().equals(interfaceEndpoint.name()) + && (interfaceEndpoint.parameterTypes().equals(endpoint.parameterTypes())); + if (nameParamsMatch) { + // whether matched method is declared on class that implements interface endpoint + Predicate isEndpointInterface = interfaceEndpoint.declaringClass() + .name()::equals; + if (endpoint.declaringClass().interfaceNames().stream().anyMatch(isEndpointInterface)) { + continue interfaceBlock; + } + } + } + String configProperty = config.denyJaxRs ? "quarkus.security.jaxrs.deny-unannotated-endpoints" + : "quarkus.security.jaxrs.default-roles-allowed"; + // this is logging only as I'm a bit worried about false positives and breaking things + // for what is very much edge case + LOG.warn("Default interface method '" + interfaceEndpoint + + "' cannot be secured with the '" + configProperty + + "' configuration property. Please implement this method for CDI " + + "interceptor binding to work"); + } } } } @@ -85,6 +122,27 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index, } } + private static void addAllUnannotatedEndpoints(CombinedIndexBuildItem index, ClassInfo classInfo, + List methods) { + if (classInfo == null) { + return; + } + addUnannotatedEndpoints(index, classInfo, methods); + if (classInfo.superClassType() != null && !classInfo.superClassType().name().equals(DotName.OBJECT_NAME)) { + addAllUnannotatedEndpoints(index, index.getIndex().getClassByName(classInfo.superClassType().name()), methods); + } + } + + private static void addUnannotatedEndpoints(CombinedIndexBuildItem index, ClassInfo classInfo, List methods) { + if (!hasSecurityAnnotation(classInfo)) { + for (MethodInfo methodInfo : classInfo.methods()) { + if (isRestEndpointMethod(index, methodInfo) && !hasSecurityAnnotation(methodInfo)) { + methods.add(methodInfo); + } + } + } + } + /** * Install the JAX-RS security provider. */ diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java index ee7c11733afc8..007fbb74b414e 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/AbstractSecurityEventTest.java @@ -37,7 +37,8 @@ public abstract class AbstractSecurityEventTest { protected static final Class[] TEST_CLASSES = { RolesAllowedResource.class, TestIdentityProvider.class, TestIdentityController.class, - UnsecuredResource.class, UnsecuredSubResource.class, EventObserver.class + UnsecuredResource.class, UnsecuredSubResource.class, EventObserver.class, UnsecuredResourceInterface.class, + UnsecuredParentResource.class }; @Inject diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java index fb13a3d50cbf6..b9e18eed5475c 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java @@ -22,8 +22,8 @@ public class DefaultRolesAllowedJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredResourceInterface.class, + TestIdentityController.class, UnsecuredParentResource.class, UnsecuredSubResource.class, HelloResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = admin\n"), "application.properties")); @@ -41,6 +41,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 200, 403, 401); } + @Test + public void shouldDenyUnannotatedOnParentClass() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 200, 403, 401); + } + + @Test + public void shouldDenyUnannotatedOnInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 200, 403, 401); + } + @Test public void shouldDenyDenyAllMethod() { String path = "/unsecured/denyAll"; diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java index 4e0fd8c7dd808..ddcc31f5ae87e 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedStarJaxRsTest.java @@ -17,8 +17,8 @@ public class DefaultRolesAllowedStarJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredParentResource.class, + TestIdentityController.class, UnsecuredResourceInterface.class, UnsecuredSubResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = **\n"), "application.properties")); diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java index 90cd2a9f77390..8ed4d8cbf445c 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java @@ -26,8 +26,8 @@ public class DenyAllJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredParentResource.class, + TestIdentityController.class, UnsecuredResourceInterface.class, UnsecuredSubResource.class, HelloResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.deny-unannotated-endpoints = true\n"), "application.properties")); @@ -58,6 +58,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 403, 401); } + @Test + public void shouldDenyUnannotatedOnParentClass() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 403, 401); + } + + @Test + public void shouldDenyUnannotatedOnInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 403, 401); + } + @Test public void shouldDenyDenyAllMethod() { String path = "/unsecured/denyAll"; diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java new file mode 100644 index 0000000000000..abf5b385e9a0d --- /dev/null +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredParentResource.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public class UnsecuredParentResource { + + @Path("/defaultSecurityParent") + @GET + public String defaultSecurityParent() { + return "defaultSecurityParent"; + } + +} diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java index 116f041ba538d..7339c9c1eda07 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResource.java @@ -12,13 +12,18 @@ * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com */ @Path("/unsecured") -public class UnsecuredResource { +public class UnsecuredResource extends UnsecuredParentResource implements UnsecuredResourceInterface { @Path("/defaultSecurity") @GET public String defaultSecurity() { return "defaultSecurity"; } + @Override + public String defaultSecurityInterface() { + return UnsecuredResourceInterface.super.defaultSecurityInterface(); + } + @Path("/permitAllPathParam/{index}") @GET @PermitAll diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java new file mode 100644 index 0000000000000..d2498d46a8c63 --- /dev/null +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/UnsecuredResourceInterface.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public interface UnsecuredResourceInterface { + + @Path("/defaultSecurityInterface") + @GET + default String defaultSecurityInterface() { + return "defaultSecurityInterface"; + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java index ff52973878de5..272d25e5b98f1 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java @@ -1,13 +1,9 @@ package io.quarkus.resteasy.reactive.common.deployment; -import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation; import static org.jboss.resteasy.reactive.common.model.ResourceInterceptor.FILTER_SOURCE_METHOD_METADATA_KEY; -import static org.jboss.resteasy.reactive.common.processor.EndpointIndexer.collectClassEndpoints; import java.io.ByteArrayInputStream; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -76,7 +72,7 @@ import io.quarkus.resteasy.reactive.spi.MessageBodyWriterOverrideBuildItem; import io.quarkus.resteasy.reactive.spi.ReaderInterceptorBuildItem; import io.quarkus.resteasy.reactive.spi.WriterInterceptorBuildItem; -import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; +import io.quarkus.security.spi.DefaultSecurityCheckBuildItem; public class ResteasyReactiveCommonProcessor { @@ -134,46 +130,13 @@ void searchForProviders(Capabilities capabilities, } @BuildStep - void setUpDenyAllJaxRs( - CombinedIndexBuildItem index, - JaxRsSecurityConfig securityConfig, - Optional resteasyDeployment, - BeanArchiveIndexBuildItem beanArchiveIndexBuildItem, - ApplicationResultBuildItem applicationResultBuildItem, - BuildProducer additionalSecuredClasses) { - - if (resteasyDeployment.isPresent() - && (securityConfig.denyJaxRs() || securityConfig.defaultRolesAllowed().isPresent())) { - final List methods = new ArrayList<>(); - Map httpAnnotationToMethod = resteasyDeployment.get().getResult().getHttpAnnotationToMethod(); - Set resourceClasses = resteasyDeployment.get().getResult().getScannedResourcePaths().keySet(); - - for (DotName className : resourceClasses) { - ClassInfo classInfo = index.getIndex().getClassByName(className); - if (classInfo == null) - throw new IllegalStateException("Unable to find class info for " + className); - if (!hasSecurityAnnotation(classInfo)) { - // collect class endpoints - Collection classEndpoints = collectClassEndpoints(classInfo, httpAnnotationToMethod, - beanArchiveIndexBuildItem.getIndex(), applicationResultBuildItem.getResult()); - - // add endpoints - for (MethodInfo classEndpoint : classEndpoints) { - if (!hasSecurityAnnotation(classEndpoint)) { - methods.add(classEndpoint); - } - } - } - } - - if (!methods.isEmpty()) { - if (securityConfig.denyJaxRs()) { - additionalSecuredClasses.produce(new AdditionalSecuredMethodsBuildItem(methods)); - } else { - additionalSecuredClasses - .produce(new AdditionalSecuredMethodsBuildItem(methods, securityConfig.defaultRolesAllowed())); - } - } + void setUpDenyAllJaxRs(JaxRsSecurityConfig securityConfig, + BuildProducer defaultSecurityCheckProducer) { + if (securityConfig.denyJaxRs()) { + defaultSecurityCheckProducer.produce(DefaultSecurityCheckBuildItem.denyAll()); + } else if (securityConfig.defaultRolesAllowed().isPresent()) { + defaultSecurityCheckProducer + .produce(DefaultSecurityCheckBuildItem.rolesAllowed(securityConfig.defaultRolesAllowed().get())); } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java index cc8d3a9db7199..1d3366fd2220a 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/AbstractSecurityEventTest.java @@ -40,7 +40,8 @@ public abstract class AbstractSecurityEventTest { protected static final Class[] TEST_CLASSES = { RolesAllowedResource.class, RolesAllowedBlockingResource.class, TestIdentityProvider.class, TestIdentityController.class, UnsecuredResource.class, UnsecuredSubResource.class, RolesAllowedService.class, - RolesAllowedServiceResource.class, EventObserver.class + RolesAllowedServiceResource.class, EventObserver.class, UnsecuredResourceInterface.class, + UnsecuredParentResource.class }; @Inject diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java index 4590ee9a9d02f..ca34474ad9052 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java @@ -18,9 +18,9 @@ public class DefaultRolesAllowedJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, + TestIdentityProvider.class, UnsecuredResourceInterface.class, TestIdentityController.class, - UnsecuredSubResource.class, HelloResource.class) + UnsecuredSubResource.class, HelloResource.class, UnsecuredParentResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed=admin\n"), "application.properties")); @@ -37,6 +37,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 200, 403, 401); } + @Test + public void shouldDenyUnannotatedParent() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 200, 403, 401); + } + + @Test + public void shouldDenyUnannotatedInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 200, 403, 401); + } + @Test public void shouldDenyDenyAllMethod() { String path = "/unsecured/denyAll"; diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java index dcad10e88e0ce..6f77ef2fff88e 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedStarJaxRsTest.java @@ -17,8 +17,8 @@ public class DefaultRolesAllowedStarJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, + TestIdentityProvider.class, UnsecuredResourceInterface.class, + TestIdentityController.class, UnsecuredParentResource.class, UnsecuredSubResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = **\n"), "application.properties")); diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java index 2e10fb07015e4..76f23ddcc8056 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java @@ -3,12 +3,25 @@ import static io.restassured.RestAssured.given; import static io.restassured.RestAssured.when; +import java.lang.reflect.Modifier; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + import org.hamcrest.Matchers; +import org.jboss.jandex.AnnotationTarget; +import org.jboss.jandex.AnnotationValue; +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.MethodInfo; +import org.jboss.resteasy.reactive.common.processor.transformation.AnnotationsTransformer; import org.jboss.shrinkwrap.api.asset.StringAsset; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.builder.BuildContext; +import io.quarkus.builder.BuildStep; +import io.quarkus.resteasy.reactive.server.spi.AnnotationsTransformerBuildItem; import io.quarkus.security.test.utils.TestIdentityController; import io.quarkus.security.test.utils.TestIdentityProvider; import io.quarkus.test.QuarkusUnitTest; @@ -21,11 +34,43 @@ public class DenyAllJaxRsTest { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(PermitAllResource.class, UnsecuredResource.class, - TestIdentityProvider.class, - TestIdentityController.class, - UnsecuredSubResource.class, HelloResource.class) + TestIdentityProvider.class, UnsecuredResourceInterface.class, + TestIdentityController.class, SpecialResource.class, + UnsecuredSubResource.class, HelloResource.class, UnsecuredParentResource.class) .addAsResource(new StringAsset("quarkus.security.jaxrs.deny-unannotated-endpoints = true\n"), - "application.properties")); + "application.properties")) + .addBuildChainCustomizer(builder -> { + builder.addBuildStep(new BuildStep() { + @Override + public void execute(BuildContext context) { + // Here we add an AnnotationsTransformer in order to make sure that the security layer + // uses the proper set of transformers + context.produce( + new AnnotationsTransformerBuildItem( + AnnotationsTransformer.builder().appliesTo(AnnotationTarget.Kind.METHOD) + .transform(transformerContext -> { + // This transformer auto-adds @GET and @Path if missing, thus emulating Renarde + MethodInfo methodInfo = transformerContext.getTarget().asMethod(); + ClassInfo declaringClass = methodInfo.declaringClass(); + if (declaringClass.name().toString().equals(SpecialResource.class.getName()) + && !methodInfo.isConstructor() + && !Modifier.isStatic(methodInfo.flags())) { + if (methodInfo.declaredAnnotation(GET.class.getName()) == null) { + // auto-add it + transformerContext.transform().add(GET.class).done(); + } + if (methodInfo.declaredAnnotation(Path.class.getName()) == null) { + // auto-add it + transformerContext.transform().add(Path.class, + AnnotationValue.createStringValue("value", + methodInfo.name())) + .done(); + } + } + }))); + } + }).produces(AnnotationsTransformerBuildItem.class).build(); + }); @BeforeAll public static void setupUsers() { @@ -40,6 +85,18 @@ public void shouldDenyUnannotated() { assertStatus(path, 403, 401); } + @Test + public void shouldDenyUnannotatedOnParentClass() { + String path = "/unsecured/defaultSecurityParent"; + assertStatus(path, 403, 401); + } + + @Test + public void shouldDenyUnannotatedOnInterface() { + String path = "/unsecured/defaultSecurityInterface"; + assertStatus(path, 403, 401); + } + @Test public void shouldDenyUnannotatedNonBlocking() { String path = "/unsecured/defaultSecurityNonBlocking"; @@ -90,6 +147,14 @@ public void testServerExceptionMapper() { .body(Matchers.equalTo("unauthorizedExceptionMapper")); } + @Test + public void shouldDenyUnannotatedWithAnnotationTransformer() { + String path = "/special/explicit"; + assertStatus(path, 403, 401); + path = "/special/implicit"; + assertStatus(path, 403, 401); + } + private void assertStatus(String path, int status, int anonStatus) { given().auth().preemptive() .basic("admin", "admin").get(path) @@ -105,4 +170,15 @@ private void assertStatus(String path, int status, int anonStatus) { } + @Path("/special") + public static class SpecialResource { + @GET + public String explicit() { + return "explicit"; + } + + public String implicit() { + return "implicit"; + } + } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java new file mode 100644 index 0000000000000..8250d5a9bf9a6 --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.reactive.server.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public class UnsecuredParentResource { + + @Path("/defaultSecurityParent") + @GET + public String defaultSecurityParent() { + return "defaultSecurityParent"; + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java index bca5025db0a5f..82622c3e585de 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java @@ -12,7 +12,7 @@ * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com */ @Path("/unsecured") -public class UnsecuredResource { +public class UnsecuredResource extends UnsecuredParentResource implements UnsecuredResourceInterface { @Path("/defaultSecurity") @GET public String defaultSecurity() { diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java new file mode 100644 index 0000000000000..7be652f15ef8d --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java @@ -0,0 +1,14 @@ +package io.quarkus.resteasy.reactive.server.test.security; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +public interface UnsecuredResourceInterface { + + @Path("/defaultSecurityInterface") + @GET + default String defaultSecurityInterface() { + return "defaultSecurityInterface"; + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java index 00b25676a4bd4..f94c369070cf0 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java @@ -66,9 +66,14 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti ResteasyReactiveResourceInfo lazyMethod = requestContext.getTarget().getLazyMethod(); MethodDescription methodDescription = lazyMethodToMethodDescription(lazyMethod); if (check == null) { - check = Arc.container().instance(SecurityCheckStorage.class).get().getSecurityCheck(methodDescription); + SecurityCheckStorage storage = Arc.container().instance(SecurityCheckStorage.class).get(); + check = storage.getSecurityCheck(methodDescription); if (check == null) { - check = NULL_SENTINEL; + if (storage.getDefaultSecurityCheck() == null || isRequestAlreadyChecked(requestContext)) { + check = NULL_SENTINEL; + } else { + check = storage.getDefaultSecurityCheck(); + } } this.check = check; } @@ -193,6 +198,13 @@ private void preventRepeatedSecurityChecks(ResteasyReactiveRequestContext reques requestContext.setProperty(STANDARD_SECURITY_CHECK_INTERCEPTOR, methodDescription); } + private boolean isRequestAlreadyChecked(ResteasyReactiveRequestContext requestContext) { + // when request has already been checked at least once (by another instance of this handler) + // then default security checks, like denied access to all JAX-RS resources by default + // shouldn't be applied; this doesn't mean security checks registered for methods shouldn't be applied + return requestContext.getProperty(STANDARD_SECURITY_CHECK_INTERCEPTOR) != null; + } + private InjectableInstance getCurrentIdentityAssociation() { InjectableInstance identityAssociation = this.currentIdentityAssociation; if (identityAssociation == null) { 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 d781085f835ac..8aa777f640b67 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 @@ -105,6 +105,7 @@ import io.quarkus.security.runtime.interceptor.SecurityHandler; import io.quarkus.security.spi.AdditionalSecuredClassesBuildItem; import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem; +import io.quarkus.security.spi.DefaultSecurityCheckBuildItem; import io.quarkus.security.spi.RolesAllowedConfigExpResolverBuildItem; import io.quarkus.security.spi.runtime.AuthorizationController; import io.quarkus.security.spi.runtime.DevModeDisabledAuthorizationController; @@ -527,6 +528,7 @@ void gatherSecurityChecks(BuildProducer syntheticBeans, BuildProducer configBuilderProducer, List additionalSecuredMethods, SecurityCheckRecorder recorder, + Optional defaultSecurityCheckBuildItem, BuildProducer reflectiveClassBuildItemBuildProducer, List additionalSecurityChecks, SecurityBuildTimeConfig config) { classPredicate.produce(new ApplicationClassPredicateBuildItem(new SecurityCheckStorageAppPredicate())); @@ -559,6 +561,15 @@ void gatherSecurityChecks(BuildProducer syntheticBeans, recorder.addMethod(builder, method.declaringClass().name().toString(), method.name(), params, methodEntry.getValue()); } + + if (defaultSecurityCheckBuildItem.isPresent()) { + var roles = defaultSecurityCheckBuildItem.get().getRolesAllowed(); + if (roles == null) { + recorder.registerDefaultSecurityCheck(builder, recorder.denyAll()); + } else { + recorder.registerDefaultSecurityCheck(builder, recorder.rolesAllowed(roles.toArray(new String[0]))); + } + } recorder.create(builder); syntheticBeans.produce( diff --git a/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java b/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java index 3cfeaa2b44b80..2d72f5f8dc2e2 100644 --- a/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java +++ b/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityCheckStorage.java @@ -10,4 +10,9 @@ default SecurityCheck getSecurityCheck(Method method) { SecurityCheck getSecurityCheck(MethodDescription methodDescription); + /** + * {@link SecurityCheck} that should be applied when there is no other check applied on incoming request. + */ + SecurityCheck getDefaultSecurityCheck(); + } 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 8661d06f3f4bd..545b3249cd9b0 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 @@ -351,4 +351,8 @@ private Class loadClass(String className) { throw new RuntimeException("Unable to load class '" + className + "' for creating permission", e); } } + + public void registerDefaultSecurityCheck(RuntimeValue builder, SecurityCheck securityCheck) { + builder.getValue().registerDefaultSecurityCheck(securityCheck); + } } diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java index 6be2f134e0057..25cecdb398e78 100644 --- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java +++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/interceptor/SecurityCheckStorageBuilder.java @@ -9,6 +9,7 @@ public class SecurityCheckStorageBuilder { private final Map securityChecks = new HashMap<>(); + private SecurityCheck defaultSecurityCheck; public void registerCheck(String className, String methodName, @@ -17,12 +18,24 @@ public void registerCheck(String className, securityChecks.put(new MethodDescription(className, methodName, parameterTypes), securityCheck); } + public void registerDefaultSecurityCheck(SecurityCheck defaultSecurityCheck) { + if (this.defaultSecurityCheck != null) { + throw new IllegalStateException("Default SecurityCheck has already been registered"); + } + this.defaultSecurityCheck = defaultSecurityCheck; + } + public SecurityCheckStorage create() { return new SecurityCheckStorage() { @Override public SecurityCheck getSecurityCheck(MethodDescription methodDescription) { return securityChecks.get(methodDescription); } + + @Override + public SecurityCheck getDefaultSecurityCheck() { + return defaultSecurityCheck; + } }; } } diff --git a/extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java b/extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java new file mode 100644 index 0000000000000..ed3dafe18de0d --- /dev/null +++ b/extensions/security/spi/src/main/java/io/quarkus/security/spi/DefaultSecurityCheckBuildItem.java @@ -0,0 +1,28 @@ +package io.quarkus.security.spi; + +import java.util.List; +import java.util.Objects; + +import io.quarkus.builder.item.SimpleBuildItem; + +public final class DefaultSecurityCheckBuildItem extends SimpleBuildItem { + + public final List rolesAllowed; + + private DefaultSecurityCheckBuildItem(List rolesAllowed) { + this.rolesAllowed = rolesAllowed; + } + + public static DefaultSecurityCheckBuildItem denyAll() { + return new DefaultSecurityCheckBuildItem(null); + } + + public static DefaultSecurityCheckBuildItem rolesAllowed(List rolesAllowed) { + Objects.requireNonNull(rolesAllowed); + return new DefaultSecurityCheckBuildItem(List.copyOf(rolesAllowed)); + } + + public List getRolesAllowed() { + return rolesAllowed; + } +} diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java index 41f8bcaa557ef..a16267eab7b54 100644 --- a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java +++ b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java @@ -422,25 +422,6 @@ protected List createEndpoints(ClassInfo currentClassInfo, return ret; } - /** - * Return endpoints defined directly on classInfo. - * - * @param classInfo resource class - * @return classInfo endpoint method info - */ - public static Collection collectClassEndpoints(ClassInfo classInfo, - Map httpAnnotationToMethod, IndexView index, ApplicationScanningResult applicationScanningResult) { - Collection endpoints = collectEndpoints(classInfo, classInfo, new HashSet<>(), new HashSet<>(), true, - httpAnnotationToMethod, index, applicationScanningResult, new AnnotationStore(null)); - Collection ret = new HashSet<>(); - for (FoundEndpoint endpoint : endpoints) { - if (endpoint.classInfo.equals(classInfo)) { - ret.add(endpoint.methodInfo); - } - } - return ret; - } - private static List collectEndpoints(ClassInfo currentClassInfo, ClassInfo actualEndpointInfo, Set seenMethods, Set existingClassNameBindings, boolean considerApplication, Map httpAnnotationToMethod, IndexView index, ApplicationScanningResult applicationScanningResult, From 92d6a7b6b66493415b508bce9f77b1cfca026c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Fri, 26 Jan 2024 18:22:49 +0100 Subject: [PATCH 15/15] Do not require RoutingContext outside or RESTEasy handler (cherry picked from commit 36bfe4cda3129e242f95d7e2b14703d531aa29e0) --- .../runtime/StandardSecurityCheckInterceptor.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java index 036ab8fcf3b74..10ad1effe0416 100644 --- a/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java +++ b/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/StandardSecurityCheckInterceptor.java @@ -17,7 +17,7 @@ import io.quarkus.security.Authenticated; import io.quarkus.security.PermissionsAllowed; import io.quarkus.security.spi.runtime.AuthorizationController; -import io.vertx.ext.web.RoutingContext; +import io.quarkus.vertx.http.runtime.CurrentVertxRequest; /** * Security checks for RBAC annotations on endpoints are done by the {@link EagerSecurityFilter}, this interceptor @@ -30,12 +30,14 @@ public abstract class StandardSecurityCheckInterceptor { AuthorizationController controller; @Inject - RoutingContext routingContext; + CurrentVertxRequest currentVertxRequest; @AroundInvoke public Object intercept(InvocationContext ic) throws Exception { - if (controller.isAuthorizationEnabled()) { - Method method = routingContext.get(EagerSecurityFilter.class.getName()); + // RoutingContext can be null if RESTEasy is used together with other stacks that do not rely on it (e.g. gRPC) + // and this is not invoked from RESTEasy route handler + if (controller.isAuthorizationEnabled() && currentVertxRequest.getCurrent() != null) { + Method method = currentVertxRequest.getCurrent().get(EagerSecurityFilter.class.getName()); if (method != null && method.equals(ic.getMethod())) { ic.getContextData().put(SECURITY_HANDLER, EXECUTED); }