OpenAPI generation: @RolesAllowed roles are sometimes missing (random behavior)

[michalvavrik]

Describe the bug

We have OIDC security provider with RESTEasy Classic and our endpoint /secured/admin is annotated with @RolesAllowed("admin"). Sometimes, generated security scheme is empty (once in a few days, you can see our CI). Interestingly, we have identical endpoint with RESTEasy Reactive and this problem occasionally occur too.

Expected behavior

OpenAPI definition must be deterministic (and ideally with roles).

Actual behavior

Most of the times, we get "/secured/admin":{"get":{"tags":["Secured Resource"],"responses":{"200":{"description":"OK","content":{"application/json":{"schema":{"type":"string"}}}},"403":{"description":"Not Allowed"},"401":{"description":"Not Authorized"}},"security":[{"SecurityScheme":["admin"]}]}}, but sometimes we get "/secured/admin":{"get":{"tags":["Secured Resource"],"responses":{"200":{"description":"OK","content":{"application/json":{"schema":{"type":"string"}}}},"403":{"description":"Not Allowed"},"401":{"description":"Not Authorized"}},"security":[{"SecurityScheme":[]}]}}

How to Reproduce?

Steps to reproduce:

1. git clone [email protected]:michalvavrik/quarkus-test-suite.git
2. cd quarkus-test-suite && git checkout reproducer/openapi-definition-generation
3. cd security/keycloak-oidc-client-extended/
4. mvn clean verify -Dit.test=OpenApiStoreSchemaIT (repeat till you experience failure, good luck)

Please checkout daily builds of our CI for failures we experienced https://github.com/quarkus-qe/quarkus-test-suite/actions/workflows/daily.yaml, notably build #770.

Output of uname -a or ver

Linux

Output of java -version

openjdk 17.0.4 2022-07-19

GraalVM version (if different from Java)

OpenJDK Runtime Environment GraalVM CE 23.2

Quarkus version or git rev

999-SNAPSHOT

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.6

Additional information

No response

[quarkus-bot]

/cc @EricWittmann (openapi), @MikeEdgar (openapi), @phillip-kruger (openapi)

[MikeEdgar]

@michalvavrik , when did this start happening? I'm curious if it coincides with #31671

[michalvavrik]

@MikeEdgar hard to say because I added this check when you removed roles from basic scheme (to make ti compliant with specs). so I don't know about 2.16

[michalvavrik]

that is I added this test right after #31671

[MikeEdgar]

@michalvavrik I haven't been able to reproduce it using the instructions you gave with your fork/branch. Looking at daily build 770, it looks like it only failed in the native test - or am I missing something?

[michalvavrik]

@MikeEdgar nah, it's not related to native/jvm mode, for example https://github.com/quarkus-qe/quarkus-test-suite/actions/runs/4463205558 (that's build 766, before I added debugging info). I'm not surprised you have not been able to reproduce it, welcome to the club :-) Sometimes things behaves differently in CI as it's naturally slower than your workstation (or it's not related to performance; like it can only happen at certain build step order, don't know) . So far, we have experienced it 3 times (770, 766. 762), which is not that little considering I added this test a week and half back. There will be no more failures as we disabled the check on Friday, you can set up similar CI on your fork if needed.

[michalvavrik]

@MikeEdgar I've got good news, it also fails with RESTEasy Reactive (so you have extra information for your debugging, and it was a JVM mode). Please see build number 779 https://github.com/quarkus-qe/quarkus-test-suite/actions/runs/4585690826/jobs/8098110705, thank you

[michalvavrik]

For reasons I explained here quarkus-qe/quarkus-test-suite#1217 (comment) I'd like to keep issue opened. WDYT @MikeEdgar ? If there won't be a fix for this, short note in docs should do. I realize this is the edge case, but it is important to have this recorder. Thank you

[michalvavrik]

hey @MikeEdgar , the very first daily build failed with disabled Basic auth. I'm afraid there are other factors that are (also?) causing this behavior. I'm going to disable tests again quarkus-qe/quarkus-test-suite#1218 as when it one test fails other runs are cancelled and it doesn't really provide us with new information about failure.

https://github.com/quarkus-qe/quarkus-test-suite/actions/runs/4921408854/jobs/8791517634 module Keycloak + OIDC Client + Extended test io.quarkus.ts.security.keycloak.oidcclient.extended.restclient.OpenApiStoreSchemaIT.testJsonOpenApiPathAccessResource you can check logs.

I still appreciate you took a time to look at this, thank you.

[michalvavrik]

Wait a minute, now that I'm thinking about it, why there is still "SecurityScheme":{"type":"http","description":"Authentication","scheme":"basic"} in generated OpenAPI definition? I don't really have time to check this week, but if you are not quicker, I'll check next week!

[MikeEdgar]

I will take a look at this today. HTTP basic should not be in the output anymore... I'll post here what I find.

[MikeEdgar]

I think that perhaps quarkus.http.auth.basic=false needs to be set in the application.properties. I was running with the debugger and that probably influences the build/runtime application of the configuration properties.

[michalvavrik]

JAR is reused if only runtime properties has changed, so that makes sense.

[michalvavrik]

Well, I don't really know at which moment the resources are generated, sorry, don't have time to think about that right now. Let's try quarkus.http.auth.basic=false in app properties as you suggested and see.

[MikeEdgar]

@michalvavrik I see the daily job had failures, but they don't seem related to this issue anymore.

The next phase for this issue will be to address the missing support for multiple security configurations.

[michalvavrik]

Sorry @MikeEdgar, I missed your comment somehow. Sounds good, sure, I'd report it here if there were any related failures.

The next phase for this issue will be to address the missing support for multiple security configurations.

I'm glad we are further now, but bug (or missing support, whatever we call it) still can affect customers, therefore I'd prefer to keep issue open. Thanks again

[michalvavrik]

@MikeEdgar fix certainly did a trick, so I'll close the issue, thanks