Added BeanArchiveIndexBuildItem to be used for searching for secured classes

[mswiderski]

…classes

generated classes such as REST resources that are intended to be secured (using @RolesAllowed annotation) are not processed by security extension and by that they are left unprotected.

This PR introduces use of BeanArchiveIndexBuildItem to get hold of all the classes including generated.

@michalszynkiewicz tagging you as we just discussed over the topic, thanks for the help on this.

[gsmet]

We need a test.

And would it be an issue for constrained classes too? @mkouba ?

[mkouba]

And would it be an issue for constrained classes too?

@gsmet what do you mean?

[mswiderski]

@gsmet added test case hope this is what is expected

[gsmet]

@mkouba I mean it's the second time we have this issue (see #2983) and I wonder if it could be an issue for other parts of the code (typically the constraint detection for the Hibernate Validator extension for instance).

In the OpenAPI patch, IIRC, we had to consider both indexes so I'm a bit surprised this time only one is OK. All the potential security annotations occurrences will be in the new one?

[mkouba]

@gsmet in this case, we're talking about the ApplicationIndexBuildItem (i.e. src/main/java). IIRC the issues you're referencing to were about having both the CombinedIndexBuildItem and BeanArchiveIndexBuildItem which is a bit different (BeanArchiveIndexBuildItem does not contain all indexes from the CombinedIndexBuildItem).

[gsmet]

(BeanArchiveIndexBuildItem does not contain all indexes from the CombinedIndexBuildItem)

That one, I ended up understanding the last time :).

My question is:

• we were using ApplicationIndexBuildItem which is clearly not sufficient
• we switched to BeanArchiveIndexBuildItem, which is better
• in this particular case, are we sure everything will be in BeanArchiveIndexBuildItem as it does not contain everything? It might as well be OK because I suppose we are only considering CDI beans but I prefer asking.

About HV, I just wanted to be sure just using the CombinedIndexBuildItem would be OK? Or if we could miss some things, for instance for generated beans?

(I'm hijacking this PR a bit, sorry, but it looks all related to me)

[mkouba]

It might as well be OK because I suppose we are only considering CDI beans but I prefer asking.

It depends. If you only need to analyze bean classes then BeanArchiveIndexBuildItem is enough. If you neeed all classes, then CombinedIndexBuildItem is the best match. However, the BeanArchiveIndexBuildItem has one useful feature - it attempts to index a class if it's missing (originally addded to include JDK classes in the index).

[mswiderski]

I don't think that the one failed job is related to the changes here.

any feedback about the test case for this PR?

[michalszynkiewicz]

@gsmet could the error be caused some stale artifacts? I think I heard of such problems before...

@mswiderski maybe it's worth to rebase the PR on top of current master and try again? I retriggered the failed job and it failed again...

[mswiderski]

@michalszynkiewicz rebased and pushed ... let's see how it goes

[mswiderski]

this time

 Failed to execute goal on project quarkus-integration-test-spring-web: Could not resolve dependencies for project io.quarkus:quarkus-integration-test-spring-web:jar:999-SNAPSHOT: Failed to collect dependencies at io.quarkus:quarkus-spring-security:jar:999-SNAPSHOT: 
Failed to read artifact descriptor for io.quarkus:quarkus-spring-security:jar:999-SNAPSHOT: Could not transfer artifact io.quarkus:quarkus-spring-security:pom:999-SNAPSHOT from/to gradle-official-repository (https://repo.gradle.org/gradle/libs-releases-local/): Failed to transfer file https://repo.gradle.org/gradle/libs-releases-local/io/quarkus/quarkus-spring-security/999-SNAPSHOT/quarkus-spring-security-999-SNAPSHOT.pom with status code 409 -> [Help 1]
[ERROR] 

again not really related

Questions addressed.

[gsmet]

@mkouba @mswiderski I'm wondering if this one should be backported to 1.0.0.Final? WDYT?

[mswiderski]

@gsmet if possible I'd opt for having it as it essentially makes Kogito generated endpoints to not be secured even though the endpoints are properly annotated.