-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Improve the OIDC/OAuth2 documentation #10653
Conversation
I've tried to interlink all the OIDC/OAuth2 docs, clarify a few things, also added a section about the roles, etc |
53b6dfd
to
ab4acac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good by I add some questions / feedbacks
This guide explains how your Quarkus application can utilize MicroProfile Json Web Token (link:https://jwt.io/[JWT]) | ||
Role-Based Access Control (link:https://en.wikipedia.org/wiki/Role-based_access_control[RBAC]) to provide | ||
secured access to the JAX-RS endpoints. | ||
This guide explains how your Quarkus application can utilize MicroProfile JWT (MP JWT) to verify https://tools.ietf.org/html/rfc7519[JSON Web Token]s, represent them as MP JWT `JsonWebToken` and provide secured access to the JAX-RS endpoints using Bearer Token Authorization and https://en.wikipedia.org/wiki/Role-based_access_control[Role-Based Access Control]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
represent them as MP JWT
JsonWebToken
Maybe give the fully qualified class name istead: represent them as
org.microprofile.i-don't-know-which-package.JsonWebToken`.
Also, security is not only for JAX-RS endpoint right ? It also works for Sevlet and reactive route.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loicmathieu thanks for the comments, sure I'll just say Quarkus HTTP endpoints
|
||
This guide demonstrates how to use the OpenID Connect Extension to protect your application using Quarkus, where authentication and authorization are based on tokens issued by OpenId Connect and OAuth 2.0 compliant Authorization Servers such as https://www.keycloak.org/about.html[Keycloak]. | ||
This guide demonstrates how to use Quarkus OpenID Connect Extension to protect your JAX-RS applications using OpenId Connect Authorization Code Flow supported by OpenId Connect compliant Authorization Servers such as https://www.keycloak.org/about.html[Keycloak]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, it's not only JAX-RS that you can protect
|
||
== Token Claims And SecurityIdentity Roles | ||
|
||
The way the roles are mapped to the SecurityIdentity roles from the verified tokens is identical to how it is done for the link:security-openid-connect#token-claims-and-securityidentity-roles[bearer tokens] with the only difference being is that ID JWT token is used as a source of the roles by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear for my what ID JWT token
is ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loicmathieu I'll change it to just ID token
, it is returned, alongside access token, as part of the OIDC code flow, https://openid.net/specs/openid-connect-core-1_0.html#TokenResponse
I agree, let's keep the oauth2 extension for opaque token only, even if it can of validate a JWT token, we should keep the documentation as simple as possible and document the best way to do things. |
ab4acac
to
dfdcad9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@loicmathieu @pedroigor Thanks, I'll be following up with more updates around the security and OIDC docs, cheers |
@sberyozkin This seems to have broken the documentation module in
I can reproduce this locally and once I do |
I can confirm that by reverting commit dfdcad9 the build works again |
Yup, saw this in one of my PRs as well |
Fixes #10363
Fixes #10587