Skip to content

Commit

Permalink
Merge pull request quarkusio#44995 from MikeEdgar/issue-40839
Browse files Browse the repository at this point in the history
OpenAPI: check super class for inherited auto-security resource methods
  • Loading branch information
gsmet authored Dec 13, 2024
2 parents da21df4 + 8f0701f commit 42cbc8c
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ private Map<String, List<String>> getRolesAllowedMethodReferences(OpenApiFiltere
.flatMap(Collection::stream)
.flatMap(t -> getMethods(t, index))
.collect(Collectors.toMap(
e -> createUniqueMethodReference(e.getKey().declaringClass(), e.getKey()),
e -> createUniqueMethodReference(e.getKey().classInfo(), e.getKey().method()),
e -> List.of(e.getValue().value().asStringArray()),
(v1, v2) -> {
if (!Objects.equals(v1, v2)) {
Expand All @@ -615,7 +615,7 @@ private List<String> getPermissionsAllowedMethodReferences(
.getAnnotations(DotName.createSimple(PermissionsAllowed.class))
.stream()
.flatMap(t -> getMethods(t, index))
.map(e -> createUniqueMethodReference(e.getKey().declaringClass(), e.getKey()))
.map(e -> createUniqueMethodReference(e.getKey().classInfo(), e.getKey().method()))
.distinct()
.toList();
}
Expand All @@ -626,18 +626,18 @@ private List<String> getAuthenticatedMethodReferences(OpenApiFilteredIndexViewBu
.getAnnotations(DotName.createSimple(Authenticated.class.getName()))
.stream()
.flatMap(t -> getMethods(t, index))
.map(e -> createUniqueMethodReference(e.getKey().declaringClass(), e.getKey()))
.map(e -> createUniqueMethodReference(e.getKey().classInfo(), e.getKey().method()))
.distinct()
.toList();
}

private static Stream<Map.Entry<MethodInfo, AnnotationInstance>> getMethods(AnnotationInstance annotation,
private static Stream<Map.Entry<ClassAndMethod, AnnotationInstance>> getMethods(AnnotationInstance annotation,
IndexView index) {
if (annotation.target().kind() == Kind.METHOD) {
MethodInfo method = annotation.target().asMethod();

if (isValidOpenAPIMethodForAutoAdd(method)) {
return Stream.of(Map.entry(method, annotation));
return Stream.of(Map.entry(new ClassAndMethod(method.declaringClass(), method), annotation));
}
} else if (annotation.target().kind() == Kind.CLASS) {
ClassInfo classInfo = annotation.target().asClass();
Expand All @@ -647,7 +647,23 @@ private static Stream<Map.Entry<MethodInfo, AnnotationInstance>> getMethods(Anno
// drop methods that specify the annotation directly
.filter(method -> !method.hasDeclaredAnnotation(annotation.name()))
.filter(method -> isValidOpenAPIMethodForAutoAdd(method))
.map(method -> Map.entry(method, annotation));
.map(method -> {
final ClassInfo resourceClass;

if (method.declaringClass().isInterface()) {
/*
* smallrye-open-api processes interfaces as the resource class as long as
* there is a concrete implementation available. Using the interface method's
* declaring class here allows us to match on the hash that will be set by
* #handleOperation during scanning.
*/
resourceClass = method.declaringClass();
} else {
resourceClass = classInfo;
}

return Map.entry(new ClassAndMethod(resourceClass, method), annotation);
});
}

return Stream.empty();
Expand Down Expand Up @@ -678,7 +694,7 @@ private Map<String, ClassAndMethod> getClassNamesMethodReferences(
.getAllKnownSubclasses(declaringClass.name()), classNames);
} else {
String ref = createUniqueMethodReference(declaringClass, method);
classNames.put(ref, new ClassAndMethod(declaringClass.simpleName(), method.name()));
classNames.put(ref, new ClassAndMethod(declaringClass, method));
}
}
}
Expand All @@ -688,16 +704,15 @@ private Map<String, ClassAndMethod> getClassNamesMethodReferences(
void addMethodImplementationClassNames(MethodInfo method, Type[] params, Collection<ClassInfo> classes,
Map<String, ClassAndMethod> classNames) {
for (ClassInfo impl : classes) {
String simpleClassName = impl.simpleName();
MethodInfo implMethod = impl.method(method.name(), params);

if (implMethod != null) {
classNames.put(createUniqueMethodReference(impl, implMethod),
new ClassAndMethod(simpleClassName, implMethod.name()));
new ClassAndMethod(impl, implMethod));
}

classNames.put(createUniqueMethodReference(impl, method),
new ClassAndMethod(simpleClassName, method.name()));
new ClassAndMethod(impl, method));
}
}

Expand Down Expand Up @@ -769,7 +784,6 @@ private static boolean isOpenAPIEndpoint(MethodInfo method) {
}

private static List<MethodInfo> getMethods(ClassInfo declaringClass, IndexView index) {

List<MethodInfo> methods = new ArrayList<>();
methods.addAll(declaringClass.methods());

Expand All @@ -782,8 +796,17 @@ private static List<MethodInfo> getMethods(ClassInfo declaringClass, IndexView i
}
}
}
return methods;

DotName superClassName = declaringClass.superName();
if (superClassName != null) {
ClassInfo superClass = index.getClassByName(superClassName);

if (superClass != null) {
methods.addAll(getMethods(superClass, index));
}
}

return methods;
}

private static Set<DotName> getAllOpenAPIEndpoints() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package io.quarkus.smallrye.openapi.deployment.filter;

public record ClassAndMethod(String className, String methodName) {
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.MethodInfo;

public record ClassAndMethod(ClassInfo classInfo, MethodInfo method) {

}
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ private void maybeSetSummaryAndTag(Operation operation, String methodRef) {

if (doAutoOperation && operation.getSummary() == null) {
// Auto add a summary
operation.setSummary(capitalizeFirstLetter(splitCamelCase(classMethod.methodName())));
operation.setSummary(capitalizeFirstLetter(splitCamelCase(classMethod.method().name())));
}

if (doAutoTag && (operation.getTags() == null || operation.getTags().isEmpty())) {
operation.addTag(splitCamelCase(classMethod.className()));
operation.addTag(splitCamelCase(classMethod.classInfo().simpleName()));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.quarkus.smallrye.openapi.test.jaxrs;

import static org.hamcrest.Matchers.notNullValue;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
Expand All @@ -8,50 +10,50 @@
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class AutoSecurityAuthenticateTestCase {
class AutoSecurityAuthenticateTestCase {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(ResourceBean2.class, OpenApiResourceAuthenticatedAtClassLevel.class,
OpenApiResourceAuthenticatedInherited1.class, OpenApiResourceAuthenticatedInherited2.class,
OpenApiResourceAuthenticatedAtMethodLevel.class, OpenApiResourceAuthenticatedAtMethodLevel2.class)
.addAsResource(
new StringAsset("quarkus.smallrye-openapi.security-scheme=jwt\n"
+ "quarkus.smallrye-openapi.security-scheme-name=JWTCompanyAuthentication\n"
+ "quarkus.smallrye-openapi.security-scheme-description=JWT Authentication"),

new StringAsset("""
quarkus.smallrye-openapi.security-scheme=jwt
quarkus.smallrye-openapi.security-scheme-name=JWTCompanyAuthentication
quarkus.smallrye-openapi.security-scheme-description=JWT Authentication
"""),
"application.properties"));

@Test
public void testAutoSecurityRequirement() {
void testAutoSecurityRequirement() {
RestAssured.given().header("Accept", "application/json")
.when().get("/q/openapi")
.then()
.log().body()
.and()
.body("components.securitySchemes.JWTCompanyAuthentication", Matchers.hasEntry("type", "http"))
.and()
.body("components.securitySchemes.JWTCompanyAuthentication",
Matchers.hasEntry("description", "JWT Authentication"))
.and()
.body("components.securitySchemes.JWTCompanyAuthentication", Matchers.hasEntry("scheme", "bearer"))
.and()
.body("components.securitySchemes.JWTCompanyAuthentication", Matchers.hasEntry("bearerFormat", "JWT"))
.and()
.body("paths.'/resource2/test-security/annotated'.get.security.JWTCompanyAuthentication",
Matchers.notNullValue())
.and()
.body("paths.'/resource2/test-security/naked'.get.security.JWTCompanyAuthentication", Matchers.notNullValue())
.and()
.body("paths.'/resource2/test-security/classLevel/1'.get.security.JWTCompanyAuthentication",
Matchers.notNullValue())
.and()
.body("paths.'/resource2/test-security/classLevel/2'.get.security.JWTCompanyAuthentication",
Matchers.notNullValue())
.and()
.body("paths.'/resource2/test-security/classLevel/3'.get.security.MyOwnName",
Matchers.notNullValue())
.and()
.body("paths.'/resource3/test-security/annotated'.get.security.AtClassLevel", Matchers.notNullValue());
.body("components.securitySchemes.JWTCompanyAuthentication", Matchers.allOf(
Matchers.hasEntry("type", "http"),
Matchers.hasEntry("description", "JWT Authentication"),
Matchers.hasEntry("scheme", "bearer"),
Matchers.hasEntry("bearerFormat", "JWT")))
.body("paths.'/resource2/test-security/annotated'.get.security.JWTCompanyAuthentication", notNullValue())
.body("paths.'/resource2/test-security/naked'.get.security.JWTCompanyAuthentication", notNullValue())
.body("paths.'/resource2/test-security/classLevel/1'.get.security.JWTCompanyAuthentication", notNullValue())
.body("paths.'/resource2/test-security/classLevel/2'.get.security.JWTCompanyAuthentication", notNullValue())
.body("paths.'/resource2/test-security/classLevel/3'.get.security.MyOwnName", notNullValue())
.body("paths.'/resource3/test-security/annotated'.get.security.AtClassLevel", notNullValue())
.body("paths.'/resource-inherited1/test-security/classLevel/1'.get.security.JWTCompanyAuthentication",
notNullValue())
.body("paths.'/resource-inherited1/test-security/classLevel/2'.get.security.JWTCompanyAuthentication",
notNullValue())
.body("paths.'/resource-inherited1/test-security/classLevel/3'.get.security.MyOwnName", notNullValue())
.body("paths.'/resource-inherited2/test-security/classLevel/1'.get.security.JWTCompanyAuthentication",
notNullValue())
.body("paths.'/resource-inherited2/test-security/classLevel/2'.get.security.CustomOverride",
notNullValue())
.body("paths.'/resource-inherited2/test-security/classLevel/3'.get.security.MyOwnName", notNullValue());

}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package io.quarkus.smallrye.openapi.test.jaxrs;

import jakarta.ws.rs.Path;

import org.eclipse.microprofile.openapi.annotations.servers.Server;
import org.eclipse.microprofile.openapi.annotations.tags.Tag;

import io.quarkus.security.Authenticated;

@Path("/resource-inherited1")
@Tag(name = "test")
@Server(url = "serverUrl")
@Authenticated
public class OpenApiResourceAuthenticatedInherited1 extends OpenApiResourceAuthenticatedAtClassLevel {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package io.quarkus.smallrye.openapi.test.jaxrs;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;

import org.eclipse.microprofile.openapi.annotations.security.SecurityRequirement;
import org.eclipse.microprofile.openapi.annotations.servers.Server;
import org.eclipse.microprofile.openapi.annotations.tags.Tag;

import io.quarkus.security.Authenticated;

@Path("/resource-inherited2")
@Tag(name = "test")
@Server(url = "serverUrl")
@Authenticated
public class OpenApiResourceAuthenticatedInherited2 extends OpenApiResourceAuthenticatedInherited1 {

@GET
@Path("/test-security/classLevel/2")
@SecurityRequirement(name = "CustomOverride")
public String secureEndpoint2() {
return "secret";
}

}

0 comments on commit 42cbc8c

Please sign in to comment.