[jaxrs-spec][quarkus] Emit @RolesAllowed({"**"}) for HTTP Basic, Bearer, api-key and OAuth2 or OpenID with empty scopes and rename "useQuarkusSecurityAnnotations" to "useJakartaSecurityAnnotations"#23680
Open
Ignacio-Vidal wants to merge 17 commits intoOpenAPITools:masterfrom
Conversation
7479247 to
5daad84
Compare
5daad84 to
9b2aa6e
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="quarkus-security-github-issue.md">
<violation number="1" location="quarkus-security-github-issue.md:59">
P1: OpenAPI scope requirements are conjunctive, but this mapping uses `@RolesAllowed` OR semantics and can under-enforce multi-scope authorization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
e92ab2e to
b3b43db
Compare
e9baa9a to
9e7f29b
Compare
Contributor
Author
|
@wing328 - any chance you could review this? I have tested this in a separate projec that loads the openapi-generator-SNAPSHOT version from my local m2:
|
e2b3964 to
1b06135
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

This is part 2 of #23691 to improve authentication and authorisation support in the
jaxrs-spec/quarkusserver generator.What this PR does
When the new
useJakartaSecurityAnnotationsflag is enabled (Quarkus library only, requiresuseJakartaEe=true), the generator emits@jakarta.annotation.security.RolesAllowed({"**"})on JAX-RS interface methods and implementation stubs for operations whose security requirements mean any authenticated user is sufficient — i.e. authentication is required but no specific role or scope is enforced.Configuration
useJakartaSecurityAnnotationsfalselibrary=quarkus,useJakartaEe=trueSetting
useJakartaSecurityAnnotations=truewithuseJakartaEe=falsefails fast withIllegalArgumentException—@jakarta.annotation.security.RolesAllowedcannot coexist with thejavax.*namespace.Semantic rules
Schemes that qualify on their own
SecurityIdentityin Quarkusscheme: [])OR semantics — at least one alternative must qualify
The OpenAPI
securityarray is OR: a request is authorised if any one of the listed alternatives is satisfied. The annotation is emitted when at least one OR alternative fully qualifies on its own:AND semantics — all co-required schemes must qualify
A single
SecurityRequirementobject with multiple keys is AND: all listed schemes must be satisfied simultaneously. If any scheme in the AND group carries explicit scopes, the combined requirement is more restrictive than "any authenticated user", so the annotation is not emitted:Anonymous OR alternative (
- {}) — least restrictive winsOpenAPI 3 allows an empty
SecurityRequirement(- {}) inside the OR list to indicate anonymous access is also acceptable. When present, the least-restrictive alternative is "no auth required", so emitting@RolesAllowed({"**"})(which forces authentication) would contradict the spec. The annotation is not emitted in this case:(
@PermitAllfor this case will land in the next PR.)Global vs per-operation security
Operations with no
securityfield inheritopenapi.security. Operations with an explicit empty list (security: []) opt out and produce no annotation, regardless of the global block.Why the annotation is not emitted when scopes are present
@RolesAllowed({"**"})means any authenticated principal. If scopes are present, the intent is to restrict to principals holding a specific role. Emitting@RolesAllowed({"**"})in that case would be too permissive and contradict the spec. Those operations are left unannotated pending a follow-up PR that will emit@RolesAllowed({scope})for the all-scoped case.Emitting both annotations on the same method is not a valid fallback: Quarkus applies them with AND semantics, making
@RolesAllowed({"**"})redundant at best and silently incorrect at worst.Implementation notes
Dedicated processor class. The security gate logic lives in
JakartaSecurityAnnotationProcessor(package-private, inorg.openapitools.codegen.languages).JavaJAXRSSpecServerCodegen.fromOperationdelegates to it viaprocessor.applyTo(op, rawOperation, openAPI). This keeps the codegen class small and lets future PRs (@RolesAllowed({scope}),@PermitAll) add new branches without further bloating it.Why
fromOperationand notpostProcessOperationsWithModels. The OpenAPIsecurityarray uses aList<SecurityRequirement>where each element is a map of scheme name → scope list. A single map entry with multiple keys is an AND group. By the timepostProcessOperationsWithModelsruns,DefaultGenerator.filterAuthMethodshas already flattened allSecurityRequirementobjects into a plainList<CodegenSecurity>, discarding which schemes were co-located in the same AND group. Evaluating mixed-scope AND groups correctly requires the raw structure, which is still available infromOperationvia theOperationparameter.processOptsordering. The flag is read aftersuper.processOpts()so that consumers who supplylibraryvia Gradle'sconfigOptions(rather than as a direct task property) are honoured — thelibraryfield is null until super resolves it fromadditionalProperties.Vendor extension. The processor sets
x-jakarta-any-roles = trueon qualifying operations. A shared partialjakartaSecurityAnnotations.mustacheis included from bothapiInterface.mustache(interface-only mode) andapiMethod.mustache(implementation stub mode), so the annotation lives in one place.Defensive logging. Spec references to undefined schemes, schemes with missing
type, or unrecognised scheme types produce aLOGGER.warnso users can diagnose generated output that lacks expected annotations.Test coverage
A single parameterised test (
quarkusEmitsExpectedRolesAllowedWildcardCount) drives a 32-row@DataProvidercovering:quarkus-oauth2-no-scopes.yaml,quarkus-oauth2-with-scopes.yaml,quarkus-oauth2-multi-flow-no-scopes.yamlquarkus-oauth2-or-empty-and-scoped.yaml,quarkus-oauth2-scoped-or-api-key.yamlquarkus-http-basic.yaml,quarkus-http-bearer.yaml,quarkus-api-key.yamlquarkus-openidconnect-no-scopes.yaml,quarkus-openidconnect-with-scopes.yamlsecurity: []opt-outquarkus-global-security-one-op-disabled.yaml,quarkus-global-oauth2-or-scoped-and-unscoped.yamlquarkus-and-group-mixed-scopes.yaml- {})quarkus-or-with-anonymous.yamlPlus targeted tests:
quarkusJakartaSecurityCoexistsWithMicroProfileAnnotations— asserts@RolesAllowedand MicroProfile@SecurityRequirementboth render on the same method (quarkus-microprofile-coexist.yaml).useJakartaSecurityIsHonouredWhenLibrarySuppliedViaAdditionalProperties— regression for the GradleconfigOptionslibrary-resolution path.useJakartaSecurityRequiresUseJakartaEe— fail-fast guard.Migration
Replace
useQuarkusSecurityAnnotationswithuseJakartaSecurityAnnotationsso the support for authentication and authorisation annotations can be extended to other libraries under the same generator. #23699 added the initialuseQuarkusSecurityAnnotationsannotation to master and has not been released in a tag version yet.Generated code now uses
@jakarta.annotation.security.RolesAllowed({"**"})instead of@io.quarkus.security.Authenticated.PR checklist
master(upcoming7.x.0minor release)."fixes #123"present in the PR description).