quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.69k stars 2.65k forks source link

Clarify usage of OIDC between web_app and service + limitations of Providers such as Microsoft when using Service (Using Microsoft to protect a Service Application) #26614

Open StephenOTT opened 2 years ago

StephenOTT commented 2 years ago

Description

The current OIDC docs explain there are predefined providers such as Microsoft, twitter, GitHub, etc. But they are for web_app usage (this is written in the docs, but VERY easy to miss). Furthermore there are significant configs needed to make these providers work when you have quarkus.oidc.application-type=service.

To make it all work with the Dev UI Swagger UI, there are even more configs (often overlapping) required.

I have figured out what is needed for the Microsoft Provider (after days of banding the head against the wall).

This was a helpful article that made sense of much of the inconsistencies in the OIDC compliance of the Azure AD: https://xsreality.medium.com/making-azure-ad-oidc-compliant-5734b70c43ff

Result:

quarkus.oidc.application-type=service

# note: Have seen some issues where using common endpoint for web_app does not run the logout redirect
#quarkus.oidc.auth-server-url=https://login.microsoftonline.com/common/v2.0/
quarkus.oidc.auth-server-url=https://login.microsoftonline.com/MY_TENANT_ID/v2.0
#!! Ensure to be using V2.0 of the endpoints by setting the accessTokenAcceptedVersion:
# ensure that your accessTokenAcceptedVersion is set to 2: AzureAd>Your App>Manifest>"accessTokenAcceptedVersion": 2
# If using the /common endpoint then you need to set quarkus.oidc.token.issuer=any as the ISS will be sts.windows
#quarkus.oidc.token.issuer=any

quarkus.oidc.client-id=MY_CLIENT_ID

# Secret is optional as MS does not provide any Introspection service so the secret is not needed for JWT validation
#quarkus.oidc.credentials.secret=MY_CLIENT_SECRET

# Mapping of the roles in the JWT:
# To assign custom roles to a user, your organization needs Azure AD Premium P1 or P2.
# The roles would be defined as App Roles, and then applied to your user's in the AD.
#quarkus.oidc.roles.role-claim-path=roles

# user-info endpoint (triggered by the user-info-require=true config) cannot be accessed with the JWT that microsoft provides:
# Get UserInfo on: https://graph.microsoft.com/oidc/userinfo
# 401, error message: {"error":{"code":"InvalidAuthenticationToken","message":"Access token validation failure. Invalid audience.
# This fails because the graph api and login.micorosoft.com have different audience values
# see https://docs.microsoft.com/en-us/azure/active-directory/develop/userinfo#notes-and-caveats-on-the-userinfo-endpoint as the limits of endpoint make it less useful / not worth it
#quarkus.oidc.authentication.user-info-required=true

See https://github.com/quarkusio/quarkus/issues/26615 for further configs on how to make swagger ui function

based on discussions with @sberyozkin , it was suggested to look at updating the docs to explain how these providers can be used for Service App protection.

I would also recommend a clarification on the docs around the OIDC configs to explain which configs are designed for web_app usage. Many of the OIDC configs say things like "List of Scopes", but miss the critical detail that it is a "web_app" config.

Implementation ideas

  1. Clarify docs for web_app vs service for the oidc configs
  2. Add example of microsoft provider config for Service App protection

Why i chose the Microsoft provider is they provide a Github login. So it provides a OIDC login that we can login with Github during testing.

It would be great if the docs were clear on usage so others do not need to spent many days++ trying to figure out the crazy nuances and we can get to building OIDC supported apps!

quarkus-bot[bot] commented 2 years ago

/cc @pedroigor, @sberyozkin

sberyozkin commented 2 years ago

@StephenOTT Thanks, but lets keep Swagger UI related concerns in a separate issue, please move it to a new issue. For Quarkus OIDC the OIDC Dev UI is recommended.

A general comment: we try our best for quarkus-oidc to support all the variations but we are not the experts in all the providers out there, so users need to help us tune the docs, provide some additional info related to the providers. Example, Also ensure that your accessTokenAcceptedVersion is set to 2: AzureAd>Your App>Manifest>"accessTokenAcceptedVersion": 2 - it is only the active Azure users who have this knowledge or can find it.

Or # user-info-required does not function: - it is a misleading statement because it can't fix the peculiarities of Microsoft APIs.

StephenOTT commented 2 years ago

@StephenOTT Thanks, but lets keep Swagger UI related concerns in a separate issue, please move it to a new issue. For Quarkus OIDC the OIDC Dev UI is recommended.

No problem! Edit: Created https://github.com/quarkusio/quarkus/issues/26615

A general comment: we try our best for quarkus-oidc to support all the variations but we are not the experts in all the providers out there, so users need to help us tune the docs, provide some additional info related to the providers. Example, Also ensure that your accessTokenAcceptedVersion is set to 2: AzureAd>Your App>Manifest>"accessTokenAcceptedVersion": 2 - it is only the active Azure users who have this knowledge or can find it.

yes agreed!

Or # user-info-required does not function: - it is a misleading statement because it can't fix the peculiarities of Microsoft APIs.

It does not function ;) when used with the configs provided, because of how MS apis work: Yes it is misleading. I will adjust the original comment to clarify. 👍

StephenOTT commented 2 years ago

original comment has been updated with clarifications.

sberyozkin commented 2 years ago

Sounds good, and np at all, there was no need to change the original one, I can be a bit sensitive if quarkus-oidc is said not to function but I can handle that at the same time :-) I agree we need to improve the well known providers docs - the version you are seeing was the very first try, but more has to be done. Thanks for your input

StephenOTT commented 2 years ago

Second issue for OIDC Swagger UI has been crated https://github.com/quarkusio/quarkus/issues/26615

sberyozkin commented 2 years ago

@StephenOTT cool, thanks

StephenOTT commented 2 years ago

@sberyozkin a good practice may be to only add additional providers when the dev shows ~docs on the needed configs for each app-type: web_app, service, and hybrid, or at least demonstrate that they do not work with XYZ app-types due to abc limits with the provider.

That should resolve any future confusion about configs and usage.

For example i see there is a open issue for a Twitch provider in #24833

sberyozkin commented 2 years ago

@StephenOTT I agree. All the documented providers should be reviewed

By the way, I shoud've added that any provider properties can be overridden, for example:

quarkus.oidc.provider=microsoft
quarkus.oidc.application-type=service
StephenOTT commented 2 years ago

Maybe it would good to add code links in the docs on each provider enum to so we can easily find the config that was used for that provider. The differences are pretty significant

StephenOTT commented 2 years ago

@StephenOTT I agree. All the documented providers should be reviewed

By the way, I shoud've added that any provider properties can be overridden, for example:

quarkus.oidc.provider=microsoft
quarkus.oidc.application-type=service

@sberyozkin using the provider=microsoft as a default and then overriding seems ~risky when flipping between web_app, service, ad hybrid. For example when you implement Microsoft the issuer is set to any; if feels like there should also me a set of configs per provider/app-type pair.

If each provider was documented/tested on its usage for web_app, service, and hybrid then I would imagine being more trustful of the implications of re-using the default web_app provider configs.