Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support @RolesAllowed values populated to openapi schema now that 3.1.0 spec supports it #37199

Closed
vickymicky opened this issue Nov 18, 2023 · 7 comments · Fixed by #44407
Closed

Comments

@vickymicky
Copy link

vickymicky commented Nov 18, 2023

Description

Issue:

@RolesAllowed annotation values are not populated to openapi schema.yaml, As a workaround, If I add
@SecurityRequirement(name = "BearerJWTScheme", scopes = { "roleA", "roleB" }) I see it populated properly but it doesnt get picked automatically even with quarkus.smallrye-openapi.auto-add-security-requirement property configured to true.

Observation:

On further analysis, I see it was allowed only for "oauth2" or "openIdConnect" security scheme due to the fact, it was not supported for other security schemes till open api 3.0.x version, as outlined in this issue, #27373

Expectation:

Support Open API spec 3.1.0

As explained in spec,

Each name MUST correspond to a security scheme which is declared in the Security Schemes under the Components Object. If the security scheme is of type "oauth2" or "openIdConnect", then the value is a list of scope names required for the execution, and the list MAY be empty if authorization does not require a specified scope. For other security scheme types, the array MAY contain a list of role names which are required for the execution, but are not otherwise defined or exchanged in-band.

Sample on how it would like in openapi spec

openapi: 3.1.0
info:
  title: Non-oAuth Scopes example
  version: 1.0.0
paths:
  /users:
    get:
      security:
        - bearerAuth:
            - 'read:users'
            - 'public'
components:
  securitySchemes:
    bearerAuth:
      type: http
      scheme: bearer
      bearerFormat: jwt
      description: 'note: non-oauth scopes are not defined at the securityScheme level'

Implementation ideas

Solution:

I would like to raise a PR to fix this by removing the condition here, https://github.com/quarkusio/quarkus/blob/main/extensions/smallrye-openapi/deployment/src/main/java/io/quarkus/smallrye/openapi/deployment/filter/AutoRolesAllowedFilter.java#L114

Since the open api version is configurable via quarkus.smallrye-openapi.open-api-version.

But it might affect the users who are not migrated to new open api version and they might get undesired schema output.

To fix this properly, I see two options,

  1. We could check against the configured open api version and disable the condition but not sure whether quarkus team wants to maintain code to support multiple versions or
  2. Wait for smallrye-openapi to start supporting 3.1.0 by default via issue mentioned on microprofile-openapi.

Looking for direction from Quarkus team on how to proceed on this ?

Copy link

quarkus-bot bot commented Nov 18, 2023

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

@sultansoy
Copy link
Contributor

guys, any updates here? @EricWittmann @MikeEdgar @phillip-kruger ?

@MikeEdgar
Copy link
Contributor

OAS 3.1.0 support will be coming with MicroProfile OpenAPI 4.0, which should be released shortly. We can look at supporting this functionality when updating Quarkus to use smallrye-open-api 4.0 (which has not yet been released either).

@michalvavrik
Copy link
Member

michalvavrik commented Nov 9, 2024

OAS 3.1.0 support will be coming with MicroProfile OpenAPI 4.0, which should be released shortly. We can look at supporting this functionality when updating Quarkus to use smallrye-open-api 4.0 (which has not yet been released either).

@MikeEdgar I know OpenAPI 4.0 is here already, I'd like to help with this one (if not done already?). Can you give me some very high level directions where I can set collected roles (that one I can do :-)) to OpenAPI? You don't need to go into any details.

@MikeEdgar
Copy link
Contributor

@michalvavrik , I think you will want to change the orElse condition here [1] to check that the openAPI.getOpenapi() is greater than or equal to "3.1.0". Or perhaps make that check the primary one, and "or else" check the security scheme type, which is the more complicated check.

[1]

boolean scopesValidForScheme = securityScheme.map(Map.Entry::getValue)
.map(SecurityScheme::getType)
.map(Set.of(SecurityScheme.Type.OAUTH2, SecurityScheme.Type.OPENIDCONNECT)::contains)
.orElse(false);

@michalvavrik
Copy link
Member

Thank you @MikeEdgar

@michalvavrik
Copy link
Member

In my tests, the org.eclipse.microprofile.openapi.models.OpenAPI#getOpenapi value returned wrong value in the io.quarkus.smallrye.openapi.deployment.filter.OperationFilter#filterOpenAPI method. I think it is updated later, because generated scheme contains correct OpenAPI version. I simply used OpenAPI version from config inside that filter. See #44407.

@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants