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

OpenAPI Auto security OIDC only works with keycloak #21126

Closed
Legion2 opened this issue Nov 1, 2021 · 12 comments · Fixed by #31671
Closed

OpenAPI Auto security OIDC only works with keycloak #21126

Legion2 opened this issue Nov 1, 2021 · 12 comments · Fixed by #31671

Comments

@Legion2
Copy link

Legion2 commented Nov 1, 2021

Describe the bug

OpenAPI Auto security is a great feature, it generates OpenAPI/Swagger config automatically based on the security configuration. However, if OIDC is used the generated OpenAPI config contains wrong hardcoded url paths.

Expected behavior

If OIDC is used (with autodiscovery), the configured/discovered urls should be used instead of hardcoded ones, which are only valid for keycloak.

Actual behavior

Currently only the base url of the authorization server can be configured, but the paths of the different endpoints are hardcoded.

How to Reproduce?

  1. create a project with oidc, OpenAPI and Swagger UI
  2. configure quarkus.oidc.auth-server-url of on oidc server which is not keycloak
  3. start dev mode
  4. open Swagger UI
  5. try to authroize in the UI
  6. you are redirected to the wrong url

Output of uname -a or ver

Linux leon-pc 5.13.0-20-generic #20-Ubuntu SMP Fri Oct 15 14:21:35 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.12" 2021-07-20
OpenJDK Runtime Environment (build 11.0.12+7-Ubuntu-0ubuntu3)
OpenJDK 64-Bit Server VM (build 11.0.12+7-Ubuntu-0ubuntu3, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.3.0.Final

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

No response

Additional information

#19680

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 1, 2021

@sberyozkin
Copy link
Member

sberyozkin commented Nov 1, 2021

@phillip-kruger OidcDevConsoleProcessor does a discovery of other endpoints, so may be something similar can be done for SwaggerUi indirectly by the OIDC scheme handler.

if this is not possible then the workaround is to require, for non-Keycloak providers, disable the OIDC auto-discovery and set the various OIDC endpoints paths manually, ex, quarkus.oidc.auth-server-url=baseurl and quarkus.oidc.authorization-path=relative-to-base-url and SwaggerUI OIDC handler would calculate the authorization endpoint address by adding 2 values...

@phillip-kruger
Copy link
Member

I'll have a look a.s.a.p. I think the difference is that OidcDevConsoleProcessor only runs in Dev mode, where as auto security runs for all modes. So some of this might have to move to runtime to remove the hardcoding of the paths, or, like you said, we would need to override it in config (that defeats the purpose of auto...)

@sberyozkin
Copy link
Member

@phillip-kruger Yeah, ideally it would just work in the auto mode

@Legion2
Copy link
Author

Legion2 commented Nov 2, 2021

Just as a note, the auto mode generates a configuration which only works out of the box with keycloak. Other provides use other "defaults" and may not work with the auto mode. E.g. instead of the implicit flow, the auth code flow (+ PKCE) must be used because the implicit flow may only return an id token, some providers need additional parameter such as audience (Auth0), which must be set in swagger UI.

@sberyozkin
Copy link
Member

@Legion2, FYI the code flow is used in OidcConsole with SwaggerUI - I've tested it with Auth0 without the audience (I think I may have set a default audience in the Auth0 application, not 100% sure now) - https://quarkus.io/guides/security-openid-connect-dev-services#dev-ui-all-oidc-providers - give it a try please.

@Legion2
Copy link
Author

Legion2 commented Nov 4, 2021

@sberyozkin the generated openapi document contains only the implicit flow, so the swagger ui can not be used if the dev ui is not available (prod):

And using Auth0 without audience is not possible if you have multiple audiences (APIs) in Auth0. Also, the audience must be set in the swagger config, not the openapi document. Currently only the client id is auto set in the swagger config:

// To autoset some security config from OIDC
private static final String OIDC_CLIENT_ID = "quarkus.oidc.client-id";

Also if code auth flow with pkce should be used, this must be configured in the swagger config.
Therefore I use the following config:

quarkus.swagger-ui.oauth-additional-query-string-params={audience: "myaudience"}
quarkus.swagger-ui.oauth-use-pkce-with-authorization-code-grant=true

@sberyozkin
Copy link
Member

sberyozkin commented Nov 4, 2021

@Legion2

@sberyozkin the generated openapi document contains only the implicit flow, so the swagger ui can not be used if the dev ui is not available (prod):

Indeed, in the prod mode one can't use Dev UI.

@StephenOTT
Copy link

StephenOTT commented Jun 3, 2022

Was there any work on this? I am using quarkus 2.9.2.final and seeing the following in DevUi swagger UI

  oidc:
    discovery-enabled: false
    auth-server-url: "https://github./login/oauth"
    application-type: web_app
    token-path: "access_token"
    user-info-path: "https://api.github.com/user"
    authentication:
      redirect-path: "/Login/githubLoginSuccess"
      id-token-required: false
      scopes:
        - "user:email"
      user-info-required: true
    authorization-path: "authorize"

image

The same occurs if you use the oidc.provider=github

@StephenOTT
Copy link

@StephenOTT
Copy link

StephenOTT commented Jun 8, 2022

For others that come across this issue.

My current workaround has been to: (For Microsoft OIDC)

quarkus.oidc.client-id=your_client_id
quarkus.oidc.credentials.secret=your_client_secret

# If the common is used, redirect does not appear to work after logout:
#quarkus.oidc.auth-server-url=https://login.microsoftonline.com/common/v2.0

quarkus.oidc.auth-server-url=https://login.microsoftonline.com/YOUR-TENANT-ID/v2.0
quarkus.oidc.application-type=web_app

quarkus.oidc.authentication.redirect-path=/
# Use this to go back to the original path:
#quarkus.oidc.authentication.restore-path-after-redirect=true

# Need this in order to get the username/email:
quarkus.oidc.authentication.scopes=profile email

# Mapping of the roles in the JWT:
quarkus.oidc.roles.role-claim-path=roles


# Need this in order to activate the logout path:
quarkus.oidc.logout.path=/logout
quarkus.oidc.logout.post-logout-path=/

Then to create a dummy endpoint that activates @Authenticated such as:

@Path("/hello")
@Authenticated
class ExampleResource(
    private val identity: SecurityIdentity,
    private val token: JsonWebToken
) {



    @GET
    @Produces(MediaType.TEXT_PLAIN)
    fun hello(): String {

        return "Hello from RESTEasy Reactive: " + identity.principal.name + token.claimNames + token.getClaim("app_displayname") + token.getClaim("family_name")
    }

Then in DevUI you can navigate to this endpoint and it will trigger the redirect to Microsoft login.

Then you navigate to Swagger UI and the token should still be in memory and allow you to call the authenticated endpoints. Swagger will still show/make it look at you are not logged in, but the endpoints will pass the bearer token.

I have also found that it works better to have the following checked in your App Registration config in Azure:

image

(you want the base localhost because of the logout redirect) also note the two checkboxes

@MikeEdgar
Copy link
Contributor

When auto-configuring the OpenAPI from the OIDC configuration, shouldn't only the openidConnectUrl be configured in the security scheme (rather than the auth, refresh, and token URLs)?

Optional<SecurityInformationBuildItem.OpenIDConnectInformation> maybeInfo = securityInformationBuildItem
.getOpenIDConnectInformation();
if (maybeInfo.isPresent()) {
SecurityInformationBuildItem.OpenIDConnectInformation info = maybeInfo.get();
AutoUrl authorizationUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/protocol/openid-connect/auth");
AutoUrl refreshUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/protocol/openid-connect/token");
AutoUrl tokenUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/protocol/openid-connect/token/introspect");
return new OpenIDConnectSecurityFilter(
config.securitySchemeName,
config.securitySchemeDescription,
authorizationUrl, refreshUrl, tokenUrl);

MikeEdgar added a commit to MikeEdgar/quarkus that referenced this issue Mar 7, 2023
- Handle method-level `@RolesAllowed` that override class-level
`@RolesAllowed` values, fixes quarkusio#30997
- Render `BaseStream<T, S>` as array of `T` in OpenAPI document,
fixes quarkusio#30248 (via smallrye-open-api 3.3.0)
- Do not place scopes in OpenAPI security requirements unless the
security scheme is OAuth2 or OIDC, fixes quarkusio#27373
- Include only OIDC discovery URL in OpenAPI when auto-security is
active, fixes quarkusio#21126

Signed-off-by: Michael Edgar <[email protected]>
MikeEdgar added a commit to MikeEdgar/quarkus that referenced this issue Mar 8, 2023
- Handle method-level `@RolesAllowed` that override class-level
`@RolesAllowed` values, fixes quarkusio#30997
- Render `BaseStream<T, S>` as array of `T` in OpenAPI document,
fixes quarkusio#30248 (via smallrye-open-api 3.3.0)
- Do not place scopes in OpenAPI security requirements unless the
security scheme is OAuth2 or OIDC, fixes quarkusio#27373
- Include only OIDC discovery URL in OpenAPI when auto-security is
active, fixes quarkusio#21126

Signed-off-by: Michael Edgar <[email protected]>
@gsmet gsmet closed this as completed in b11fae5 Mar 9, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants