Fix #933 - Filter oauth security definitions from spec when generatin…

…g `CompositeAuthenticationProvider` (#942) * Fix #933 - Filter oauth security definitions from spec when generating Signed-off-by: Ricardo Zanini <[email protected]> * Make 'resolve' more complex by searching for assignable parameters Signed-off-by: Ricardo Zanini <[email protected]> * Incorporating @hbelmiro's review Signed-off-by: Ricardo Zanini <[email protected]> --------- Signed-off-by: Ricardo Zanini <[email protected]>
quarkiverse · Jan 20, 2025 · a645b3c · a645b3c
1 parent 07c812a
commit a645b3c
Show file tree

Hide file tree

Showing 4 changed files with 148 additions and 5 deletions.
diff --git a/...n/java/io/quarkiverse/openapi/generator/deployment/template/OpenApiNamespaceResolver.java b/...n/java/io/quarkiverse/openapi/generator/deployment/template/OpenApiNamespaceResolver.java
@@ -1,12 +1,19 @@
 package io.quarkiverse.openapi.generator.deployment.template;
 
 import java.io.File;
+import java.lang.reflect.Method;
 import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
 import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
 
+import org.openapitools.codegen.CodegenSecurity;
 import org.openapitools.codegen.model.OperationMap;
 
 import io.quarkiverse.openapi.generator.deployment.codegen.OpenApiGeneratorOutputPaths;
@@ -32,6 +39,7 @@ private OpenApiNamespaceResolver() {
      * @param codegenConfig Map with the model codegen properties
      * @return true if the given model class should generate the deprecated attributes
      */
+    @SuppressWarnings("unused")
     public boolean genDeprecatedModelAttr(final String pkg, final String classname,
             final HashMap<String, Object> codegenConfig) {
         final String key = String.format("%s.%s.%s", pkg, classname, GENERATE_DEPRECATED_PROP);
@@ -44,38 +52,92 @@ public boolean genDeprecatedModelAttr(final String pkg, final String classname,
      * @param codegenConfig Map with the model codegen properties
      * @return true if the given model class should generate the deprecated attributes
      */
+    @SuppressWarnings("unused")
     public boolean genDeprecatedApiAttr(final String pkg, final String classname,
             final HashMap<String, Object> codegenConfig) {
         final String key = String.format("%s.%s.%s", pkg, classname, GENERATE_DEPRECATED_PROP);
         return Boolean.parseBoolean(codegenConfig.getOrDefault(key, "true").toString());
     }
 
+    @SuppressWarnings("unused")
     public String parseUri(String uri) {
         return OpenApiGeneratorOutputPaths.getRelativePath(Path.of(uri)).toString().replace(File.separatorChar, '/');
     }
 
+    @SuppressWarnings("unused")
     public boolean hasAuthMethods(OperationMap operations) {
         return operations != null && operations.getOperation().stream().anyMatch(operation -> operation.hasAuthMethods);
     }
 
+    /**
+     * Ignore the OAuth flows by filtering every oauth instance by name. The inner openapi-generator library duplicates the
+     * OAuth instances per flow in the openapi spec.
+     * So a specification file with more than one flow defined has two entries in the list. For now, we do not use this
+     * information in runtime so it can be safely filtered and ignored.
+     *
+     * @param oauthOperations passed through the Qute template
+     * @see "resources/templates/libraries/microprofile/auth/compositeAuthenticationProvider.qute"
+     * @return The list filtered by unique auth name
+     */
+    @SuppressWarnings("unused")
+    public List<CodegenSecurity> getUniqueOAuthOperations(List<CodegenSecurity> oauthOperations) {
+        if (oauthOperations != null) {
+            return new ArrayList<>(oauthOperations.stream()
+                    .collect(Collectors.toMap(security -> security.name, security -> security,
+                            (existing, replacement) -> existing, LinkedHashMap::new))
+                    .values());
+        }
+        return Collections.emptyList();
+    }
+
     @Override
     public CompletionStage<Object> resolve(EvalContext context) {
         try {
-            Class<?>[] classArgs = new Class[context.getParams().size()];
             Object[] args = new Object[context.getParams().size()];
+            Class<?>[] classArgs = new Class[context.getParams().size()];
+
             int i = 0;
             for (Expression expr : context.getParams()) {
                 args[i] = context.evaluate(expr).toCompletableFuture().get();
                 classArgs[i] = args[i].getClass();
                 i++;
             }
-            return CompletableFuture
-                    .completedFuture(this.getClass().getMethod(context.getName(), classArgs).invoke(this, args));
+
+            Method targetMethod = findCompatibleMethod(context.getName(), classArgs);
+            if (targetMethod == null) {
+                throw new NoSuchMethodException("No compatible method found for: " + context.getName());
+            }
+
+            return CompletableFuture.completedFuture(targetMethod.invoke(this, args));
         } catch (ReflectiveOperationException | InterruptedException | ExecutionException ex) {
             return CompletableFuture.failedStage(ex);
         }
     }
 
+    private Method findCompatibleMethod(String methodName, Class<?>[] argTypes) {
+        for (Method method : this.getClass().getMethods()) {
+            if (method.getName().equals(methodName)) {
+                Class<?>[] paramTypes = method.getParameterTypes();
+                if (isAssignable(paramTypes, argTypes)) {
+                    return method;
+                }
+            }
+        }
+        return null;
+    }
+
+    private boolean isAssignable(Class<?>[] paramTypes, Class<?>[] argTypes) {
+        if (paramTypes.length != argTypes.length) {
+            return false;
+        }
+        for (int i = 0; i < paramTypes.length; i++) {
+            if (!paramTypes[i].isAssignableFrom(argTypes[i])) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     @Override
     public String getNamespace() {
         return "openapi";

diff --git a/...main/resources/templates/libraries/microprofile/auth/compositeAuthenticationProvider.qute b/...main/resources/templates/libraries/microprofile/auth/compositeAuthenticationProvider.qute
@@ -1,8 +1,8 @@
 package {apiPackage}.auth;
 
 @jakarta.annotation.Priority(jakarta.ws.rs.Priorities.AUTHENTICATION)
-{#for auth in oauthMethods.orEmpty}
-@io.quarkiverse.openapi.generator.markers.OauthAuthenticationMarker(name="{auth.name}", openApiSpecId="{configKey}")
+{#for auth in openapi:getUniqueOAuthOperations(oauthMethods.orEmpty)}
+@io.quarkiverse.openapi.generator.markers.OauthAuthenticationMarker(name="{auth.name}", openApiSpecId="{quarkus-generator.openApiSpecId}")
 {/for}
 {#for auth in httpBasicMethods.orEmpty}
 @io.quarkiverse.openapi.generator.markers.BasicAuthenticationMarker(name="{auth.name}", openApiSpecId="{quarkus-generator.openApiSpecId}")

diff --git a/...o/quarkiverse/openapi/generator/deployment/wrapper/OpenApiClientGeneratorWrapperTest.java b/...o/quarkiverse/openapi/generator/deployment/wrapper/OpenApiClientGeneratorWrapperTest.java
@@ -33,6 +33,8 @@
 import com.github.javaparser.ast.body.Parameter;
 import com.github.javaparser.ast.body.VariableDeclarator;
 import com.github.javaparser.ast.expr.AnnotationExpr;
+import com.github.javaparser.ast.expr.Expression;
+import com.github.javaparser.ast.expr.MemberValuePair;
 import com.github.javaparser.ast.expr.SimpleName;
 import com.github.javaparser.ast.expr.SingleMemberAnnotationExpr;
 import com.github.javaparser.ast.nodeTypes.NodeWithName;
@@ -50,6 +52,38 @@ private static Optional<MethodDeclaration> getMethodDeclarationByIdentifier(List
         return methodDeclarations.stream().filter(md -> md.getName().getIdentifier().equals(methodName)).findAny();
     }
 
+    @Test
+    void verifyOAuthDuplicateAnnotationOnCompositeAuthProvider() throws URISyntaxException, FileNotFoundException {
+        OpenApiClientGeneratorWrapper generatorWrapper = createGeneratorWrapper("issue-933-security.yaml");
+        final List<File> generatedFiles = generatorWrapper.generate("org.issue933");
+
+        assertNotNull(generatedFiles);
+        assertFalse(generatedFiles.isEmpty());
+
+        final Optional<File> authProviderFile = generatedFiles.stream()
+                .filter(f -> f.getName().endsWith("CompositeAuthenticationProvider.java")).findFirst();
+        assertThat(authProviderFile).isPresent();
+
+        CompilationUnit compilationUnit = StaticJavaParser.parse(authProviderFile.orElseThrow());
+        // Get the class declaration
+        ClassOrInterfaceDeclaration classDeclaration = compilationUnit.findFirst(ClassOrInterfaceDeclaration.class)
+                .orElseThrow(() -> new AssertionError("Class not found in the file"));
+
+        // Collect all OauthAuthenticationMarker annotations
+        long oauthAnnotationsCount = classDeclaration.getAnnotations().stream()
+                .filter(annotation -> annotation.getNameAsString()
+                        .equals("io.quarkiverse.openapi.generator.markers.OauthAuthenticationMarker"))
+                .filter(Expression::isNormalAnnotationExpr)
+                .filter(annotation -> annotation
+                        .findFirst(MemberValuePair.class,
+                                pair -> pair.getNameAsString().equals("name") && pair.getValue().toString().equals("\"oauth\""))
+                        .isPresent())
+                .count();
+
+        // Assert that there's exactly one annotation with name=oauth
+        assertThat(oauthAnnotationsCount).isEqualTo(1);
+    }
+
     @Test
     void verifyDiscriminatorGeneration() throws java.net.URISyntaxException, FileNotFoundException {
         OpenApiClientGeneratorWrapper generatorWrapper = createGeneratorWrapper("issue-852.json");

diff --git a/client/deployment/src/test/resources/openapi/issue-933-security.yaml b/client/deployment/src/test/resources/openapi/issue-933-security.yaml
@@ -0,0 +1,47 @@
+openapi: 3.0.3
+info:
+  title: Generated API
+  version: "1.0"
+paths:
+  /:
+    post:
+      operationId: doOperation
+      security:
+        - client_id: [ ]
+        - oauth: [ read, write ]
+        - bearerAuth: [ ]
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/MultiplicationOperation'
+      responses:
+        "200":
+          description: OK
+components:
+  schemas:
+    MultiplicationOperation:
+      type: object
+  securitySchemes:
+    client_id:
+      type: apiKey
+      in: header
+      name: X-Client-Id
+      x-key-type: clientId
+    bearerAuth:
+      type: http
+      scheme: bearer
+    oauth:
+      type: oauth2
+      flows:
+        authorizationCode:
+          authorizationUrl: https://example.com/oauth/authorize
+          tokenUrl: https://example.com/oauth/token
+          scopes:
+            read: Grants read access
+            write: Grants write access
+            admin: Grants read and write access to administrative information
+        clientCredentials:
+          tokenUrl: http://localhost:8382/oauth/token
+          scopes:
+            read: read