Merge pull request quarkusio#36183 from sberyozkin/jwt_jsonstr_singleton

sberyozkin · web-flow · commit ebe4832cab65 · 2023-10-02T18:16:17.000+01:00
Fail smallrye-jwt build if @claim types are injected without ClaimValue in ApplicationScoped beans
diff --git a/extensions/smallrye-jwt/deployment/src/main/java/io/quarkus/smallrye/jwt/deployment/SmallRyeJwtProcessor.java b/extensions/smallrye-jwt/deployment/src/main/java/io/quarkus/smallrye/jwt/deployment/SmallRyeJwtProcessor.java
@@ -5,9 +5,12 @@
 import java.util.Set;
 import java.util.function.BooleanSupplier;
 
+import jakarta.enterprise.context.RequestScoped;
+
 import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.ConfigProvider;
 import org.eclipse.microprofile.jwt.Claim;
+import org.eclipse.microprofile.jwt.ClaimValue;
 import org.eclipse.microprofile.jwt.Claims;
 import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.AnnotationValue;
@@ -19,6 +22,7 @@
 import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem;
 import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem.BeanConfiguratorBuildItem;
 import io.quarkus.arc.processor.BeanConfigurator;
+import io.quarkus.arc.processor.BeanInfo;
 import io.quarkus.arc.processor.BuildExtension;
 import io.quarkus.arc.processor.BuiltinScope;
 import io.quarkus.arc.processor.DotNames;
@@ -61,6 +65,12 @@ class SmallRyeJwtProcessor {
     private static final DotName CLAIM_NAME = DotName.createSimple(Claim.class.getName());
     private static final DotName CLAIMS_NAME = DotName.createSimple(Claims.class.getName());
 
+    private static final DotName CLAIM_VALUE_NAME = DotName.createSimple(ClaimValue.class);
+    private static final DotName REQUEST_SCOPED_NAME = DotName.createSimple(RequestScoped.class);
+
+    private static final Set<DotName> ALL_PROVIDER_NAMES = Set.of(DotNames.PROVIDER, DotNames.INSTANCE,
+            DotNames.INJECTABLE_INSTANCE);
+
     SmallRyeJwtBuildTimeConfig config;
 
     @BuildStep(onlyIf = IsEnabled.class)
@@ -161,14 +171,29 @@ void registerOptionalClaimProducer(BeanRegistrationPhaseBuildItem beanRegistrati
                 continue;
             }
             AnnotationInstance claimQualifier = injectionPoint.getRequiredQualifier(CLAIM_NAME);
-            if (claimQualifier != null && injectionPoint.getType().name().equals(DotNames.PROVIDER)) {
-                // Classes from jakarta.json are handled specially
+            if (claimQualifier != null) {
                 Type actualType = injectionPoint.getRequiredType();
-                if (actualType.name().equals(DotNames.OPTIONAL) && !actualType.name().toString()
-                        .startsWith("jakarta.json")) {
+
+                Optional<BeanInfo> bean = injectionPoint.getTargetBean();
+                if (bean.isPresent()) {
+                    DotName scope = bean.get().getScope().getDotName();
+                    if (!REQUEST_SCOPED_NAME.equals(scope)
+                            && (!ALL_PROVIDER_NAMES.contains(injectionPoint.getType().name())
+                                    && !CLAIM_VALUE_NAME.equals(actualType.name()))) {
+                        String error = String.format(
+                                "%s type can not be used to represent JWT claims in @Singleton or @ApplicationScoped beans"
+                                        + ", make the bean @RequestScoped or wrap this type with org.eclipse.microprofile.jwt.ClaimValue"
+                                        + " or jakarta.inject.Provider or jakarta.enterprise.inject.Instance",
+                                actualType.name());
+                        throw new IllegalStateException(error);
+                    }
+                }
+
+                if (injectionPoint.getType().name().equals(DotNames.PROVIDER) && actualType.name().equals(DotNames.OPTIONAL)) {
                     additionalTypes.add(actualType);
                 }
             }
+
         }
 
         // Register a custom bean
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/ClaimsSingletonEndpoint.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/ClaimsSingletonEndpoint.java
@@ -0,0 +1,40 @@
+package io.quarkus.jwt.test;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import jakarta.enterprise.inject.Instance;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.json.JsonString;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
+
+import org.eclipse.microprofile.jwt.Claim;
+import org.eclipse.microprofile.jwt.ClaimValue;
+import org.eclipse.microprofile.jwt.Claims;
+
+import io.quarkus.security.Authenticated;
+
+@Path("/endp")
+@Authenticated
+public class ClaimsSingletonEndpoint {
+    @Inject
+    @Claim(standard = Claims.upn)
+    ClaimValue<JsonString> upn;
+    @Inject
+    @Claim(standard = Claims.raw_token)
+    Instance<String> rawToken;
+    @Inject
+    @Claim(standard = Claims.groups)
+    Provider<Set<String>> groups;
+
+    @GET
+    @Path("claims")
+    @Produces(MediaType.TEXT_PLAIN)
+    public String verifyGroups() {
+        return upn.getValue().getString() + ":" + groups.get().stream().collect(Collectors.joining(",")) + ":" + rawToken.get();
+    }
+}
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/ClaimsSingletonTest.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/ClaimsSingletonTest.java
@@ -0,0 +1,43 @@
+package io.quarkus.jwt.test;
+
+import static org.hamcrest.Matchers.equalTo;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+import io.restassured.RestAssured;
+import io.smallrye.jwt.build.Jwt;
+
+public class ClaimsSingletonTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClass(ClaimsSingletonEndpoint.class)
+                    .addAsResource("jwtPublicKey.pem")
+                    .addAsResource("jwtPrivateKey.pem")
+                    .addAsResource("applicationJwtSign.properties", "application.properties"));
+
+    @Test
+    public void verify() throws Exception {
+        String token1 = generateToken("alice", "user");
+        RestAssured.given().auth()
+                .oauth2(token1)
+                .when().get("/endp/claims")
+                .then()
+                .statusCode(200).body(equalTo("alice:user:" + token1));
+        String token2 = generateToken("bob", "admin");
+        RestAssured.given().auth()
+                .oauth2(token2)
+                .when().get("/endp/claims")
+                .then()
+                .statusCode(200).body(equalTo("bob:admin:" + token2));
+    }
+
+    private String generateToken(String upn, String role) {
+        return Jwt.upn(upn).issuer("immo-jwt").groups(role).sign();
+    }
+}
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/DefaultScopedEndpoint.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/DefaultScopedEndpoint.java
@@ -1,7 +1,5 @@
 package io.quarkus.jwt.test;
 
-import java.util.Optional;
-
 import jakarta.annotation.security.RolesAllowed;
 import jakarta.inject.Inject;
 import jakarta.json.Json;
@@ -16,6 +14,7 @@
 import jakarta.ws.rs.core.SecurityContext;
 
 import org.eclipse.microprofile.jwt.Claim;
+import org.eclipse.microprofile.jwt.ClaimValue;
 import org.eclipse.microprofile.jwt.Claims;
 
 /**
@@ -25,7 +24,7 @@
 public class DefaultScopedEndpoint {
     @Inject
     @Claim(standard = Claims.preferred_username)
-    Optional<JsonString> currentUsername;
+    ClaimValue<JsonString> currentUsername;
     @Context
     private SecurityContext context;
 
@@ -42,13 +41,13 @@ public class DefaultScopedEndpoint {
     public JsonObject validateUsername(@QueryParam("username") String username) {
         boolean pass = false;
         String msg;
-        if (!currentUsername.isPresent()) {
+        if (currentUsername.getValue() == null) {
             msg = "Injected preferred_username value is null, FAIL";
-        } else if (currentUsername.get().getString().equals(username)) {
+        } else if (currentUsername.getValue().getString().equals(username)) {
             msg = "\nInjected Principal#getName matches, PASS";
             pass = true;
         } else {
-            msg = String.format("Injected preferred_username %s != %s, FAIL", currentUsername.get().getString(), username);
+            msg = String.format("Injected preferred_username %s != %s, FAIL", currentUsername.getValue().getString(), username);
         }
 
         JsonObject result = Json.createObjectBuilder()
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeApplicationScopedBeanTest.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeApplicationScopedBeanTest.java
@@ -0,0 +1,27 @@
+package io.quarkus.jwt.test;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OptionalTypeApplicationScopedBeanTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest test = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClass(OptionalTypeApplicationScopedEndpoint.class))
+            .assertException(t -> {
+                assertTrue(t.getMessage().startsWith(
+                        "java.util.Optional type can not be used to represent JWT claims in @Singleton or @ApplicationScoped beans, make the bean @RequestScoped"
+                                + " or wrap this type with"));
+            });
+
+    @Test
+    public void test() {
+        Assertions.fail();
+    }
+}
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeApplicationScopedEndpoint.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeApplicationScopedEndpoint.java
@@ -0,0 +1,23 @@
+package io.quarkus.jwt.test;
+
+import java.util.Optional;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+import jakarta.json.JsonString;
+import jakarta.ws.rs.GET;
+
+import org.eclipse.microprofile.jwt.Claim;
+import org.eclipse.microprofile.jwt.Claims;
+
+@ApplicationScoped
+public class OptionalTypeApplicationScopedEndpoint {
+    @Inject
+    @Claim(standard = Claims.upn)
+    Optional<JsonString> upn;
+
+    @GET
+    public String get() {
+        return "hello";
+    }
+}
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeSingletonBeanTest.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeSingletonBeanTest.java
@@ -0,0 +1,27 @@
+package io.quarkus.jwt.test;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OptionalTypeSingletonBeanTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest test = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClass(OptionalTypeSingletonEndpoint.class))
+            .assertException(t -> {
+                assertTrue(t.getMessage().startsWith(
+                        "java.util.Optional type can not be used to represent JWT claims in @Singleton or @ApplicationScoped beans, make the bean @RequestScoped"
+                                + " or wrap this type with"));
+            });
+
+    @Test
+    public void test() {
+        Assertions.fail();
+    }
+}
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeSingletonEndpoint.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/OptionalTypeSingletonEndpoint.java
@@ -0,0 +1,23 @@
+package io.quarkus.jwt.test;
+
+import java.util.Optional;
+
+import jakarta.inject.Inject;
+import jakarta.json.JsonString;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+
+import org.eclipse.microprofile.jwt.Claim;
+import org.eclipse.microprofile.jwt.Claims;
+
+@Path("/endpoint")
+public class OptionalTypeSingletonEndpoint {
+    @Inject
+    @Claim(standard = Claims.upn)
+    Optional<JsonString> upn;
+
+    @GET
+    public String get() {
+        return "hello";
+    }
+}
diff --git a/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/ScopingUnitTest.java b/extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/ScopingUnitTest.java
@@ -48,18 +48,6 @@ public void verifyUsernameClaim() throws Exception {
         Assertions.assertTrue(reply.getBoolean("pass"), reply.getString("msg"));
 
         String token2 = TokenUtils.generateTokenString("/Token2.json");
-        Response response2 = RestAssured.given().auth()
-                .oauth2(token2)
-                .when()
-                // We expect the injected preferred_username claim to still be jdoe due to default scope = @ApplicationScoped
-                .queryParam("username", "jdoe")
-                .get("/endp-defaultscoped/validateUsername").andReturn();
-
-        Assertions.assertEquals(HttpURLConnection.HTTP_OK, response2.getStatusCode());
-        String replyString2 = response2.body().asString();
-        JsonReader jsonReader2 = Json.createReader(new StringReader(replyString2));
-        JsonObject reply2 = jsonReader2.readObject();
-        Assertions.assertTrue(reply2.getBoolean("pass"), reply2.getString("msg"));
 
         Response response3 = RestAssured.given().auth()
                 .oauth2(token)
