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

Improve security guides (or impl) for Quarkus users #2231

Closed
12 tasks
emmanuelbernard opened this issue Apr 26, 2019 · 26 comments
Closed
12 tasks

Improve security guides (or impl) for Quarkus users #2231

emmanuelbernard opened this issue Apr 26, 2019 · 26 comments
Labels
area/security triage/out-of-date This issue/PR is no longer valid or relevant

Comments

@emmanuelbernard
Copy link
Member

emmanuelbernard commented Apr 26, 2019

I have been discussing with @stephanj about Quarkus and security recently.
He pointed me to the following blog post. He found the explanations and features great.
https://piotrminkowski.wordpress.com/2019/04/25/micronaut-tutorial-security/

We have likely all the features but maybe we don't explain them as nicely.
Today we have a general security guide https://quarkus.io/guides/security-guide Which is quite good I think albeit more technical than the blog. BTW, I could not figure out the reason for having a @DenyAll annotation on a resource.

Then this guide mentions JWT independently from the JWT guide
https://quarkus.io/guides/jwt-guide we have.

Then we have the keycloak-guide which is not referenced by the JWT guide.
https://quarkus.io/guides/keycloak-guide

In the blog post, the person writes a client side test which If find brilliant as it helps for the extra mile of comprehension.

So for me there at the following possible actions, all are genuine questions we should discuss and execute one once agreed:

  • 1. merge the guides into two or maybe one
  • 2. otherwise reference them and define a reading order and avoid some duplication, maybe via a uber security guide pointing to the specific sub parts.
  • 3. move the configuration list int he JWT guide as part of the steps. Right know it's complexity in your face and I don't even know what JWT is
  • 4. should we unify and have a single quarkus-security-extension or everything under quarkus-security-*
  • 5. why are the JWT properties a mix of quarkus. and mp. Ideally, we want the necessary properties to be quarkus. with possible more advanced features under another namespace. This is roughly what we did with Hibernate ORM.
  • 6. I want to know when to use normal security, when to use JWT, when to use Keycloak
  • 7. there seems to be difference into what is secured by default and what it not depending whether you use security, jwt or keycloak. I wonder if it's jsut me reading to fast or if that something we can address
  • 8. the JWT guide describe the concept of an issuer, a one sentence definition would be helpful
  • 9. now that we have keycloak integration, should we still explain how to generate a JWT token from the app with Nimbus? (me think probably but make it clearer that you don't ahve to suffer all this)
  • 10. Should the keycloak client lib have a "test mode" where it can do the JWT generation as described in 9.?
  • 11. Show the code for the client side in the guides
  • 12. Document how to do OAuth (with Twitter or Google) / with and without Keycloak
@stephanj
Copy link

In addition, an example article/document where OAuth is used (with Twitter or Google) would be really helpful. And if I can stretch my luck... one with and without KeyCloak :)

@sebastienblanc
Copy link
Contributor

  • 1. merge the guides into two or maybe one
  • 2. otherwise reference them and define a reading order and avoid some duplication, maybe via a uber security guide pointing to the specific sub parts.

I agree that some consolidation must happen around these 3 guides but not an easy task. As you said, let's start to identify the common parts like :

  • The explanation of the annotation @RolesAllowed , @PermitAll etc ..
  • The common configuration (but this needs also some refactoring like you mention in 5)

Then we could make specific sections for basic (or elytron this must also been clarified) , jwt, keycloak.

  • 3. move the configuration list int he JWT guide as part of the steps. Right know it's complexity in your face and I don't even know what JWT is

It is in the steps, looks at the section Configuring the Smallrye JWT Extension Security Information

  • 4. should we unify and have a single quarkus-security-extension or everything under quarkus-security-*
  • 5. why are the JWT properties a mix of quarkus. and mp. Ideally, we want the necessary properties to be quarkus. with possible more advanced features under another namespace. This is roughly what we did with Hibernate ORM.

Agreed

  • 6. I want to know when to use normal security, when to use JWT, when to use Keycloak

That can be hard to explain "when" because it really depends. But we could clearly explain the differences between these 3 options.

  • 7. there seems to be difference into what is secured by default and what it not depending whether you use security, jwt or keycloak. I wonder if it's jsut me reading to fast or if that something we can address

I don't think so, it's all based on the same annotation on the resources. The one exception with the keycloak extension is when you enforce authorization where the secured resources are defined on the keycloak server (but this is not mandatory approach, you can just stick to simple authentication and use the annotation on the resources.

But since it was not clear for you, we need to clarify that in the documentation.

  • 8. the JWT guide describe the concept of an issuer, a one sentence definition would be helpful

+1

  • 9. now that we have keycloak integration, should we still explain how to generate a JWT token from the app with Nimbus? (me think probably but make it clearer that you don't ahve to suffer all this)
  • 10. Should the keycloak client lib have a "test mode" where it can do the JWT generation as described in 9.?

That also needs to be clarified : the keycloak extension does not generate a JWT , like the jwt extension it verifies an incoming JWT token. The benefit of using is that extension is that it can do more than the jwt extension : it's a full OIDC lib/adapter , he also adds authorization support.

In a lot of cases, you can just use the jwt extension. The token can be issued by any idm server , like Keycloak server. You can take a look at my example here : https://github.com/sebastienblanc/quarkus-quickstart and I think @starksm64 is also writing a new quickstart showing this.

  • 11. Show the code for the client side in the guides

Instead of showing curl commands ? That would be nice but also will be a bit out of scope of Quarkus, and what frontend techno will you show : vanilla JS, Angular, Vue, React ?

@emmanuelbernard
Copy link
Member Author

     3. move the configuration list int he JWT guide as part of the steps. Right know it's complexity in your face and I don't even know what JWT is

It is in the steps, looks at the section Configuring the Smallrye JWT Extension Security Information

The first thing I see in the guide is the fat configuration table (see https://quarkus.io/guides/jwt-guide)

@emmanuelbernard
Copy link
Member Author

  1. Should the keycloak client lib have a "test mode" where it can do the JWT generation as described in 9.?

That also needs to be clarified : the keycloak extension does not generate a JWT , like the jwt extension it verifies an incoming JWT token. The benefit of using is that extension is that it can do more than the jwt extension : it's a full OIDC lib/adapter , he also adds authorization support.

In a lot of cases, you can just use the jwt extension. The token can be issued by any idm server , like Keycloak server. You can take a look at my example here : https://github.com/sebastienblanc/quarkus-quickstart and I think @starksm64 is also writing a new quickstart showing this.

I do understand that it's not the Keycloak nor the JWT extension core feature to generate a JWT token.
I was just exploring 1. why it is described in the guide if you don't need to 2. how to test things either as one shot or in a automated test. So I was thinking that the keycloak extension could simulate a JWT token and pass it without needing a server. Maybe that's not useful but that was my train of thought

@sebastienblanc
Copy link
Contributor

     3. move the configuration list int he JWT guide as part of the steps. Right know it's complexity in your face and I don't even know what JWT is

It is in the steps, looks at the section Configuring the Smallrye JWT Extension Security Information

The first thing I see in the guide is the fat configuration table (see https://quarkus.io/guides/jwt-guide)

Yeah maybe it should be at the end of the guide (or moved to a general Quarkus Reference Documentation when there will be one)

@emmanuelbernard
Copy link
Member Author

     11. Show the code for the client side in the guides

Instead of showing curl commands ? That would be nice but also will be a bit out of scope of Quarkus, and what frontend techno will you show : vanilla JS, Angular, Vue, React ?

I mean a Quarkus REST client talking to a secured service.

@emmanuelbernard
Copy link
Member Author

     3. move the configuration list int he JWT guide as part of the steps. Right know it's complexity in your face and I don't even know what JWT is

It is in the steps, looks at the section Configuring the Smallrye JWT Extension Security Information

The first thing I see in the guide is the fat configuration table (see https://quarkus.io/guides/jwt-guide)

Yeah maybe it should be at the end of the guide (or moved to a general Quarkus Reference Documentation when there will be one)

Not necessarily the end but after the "getting started" proper.Like this for example https://quarkus.io/guides/hibernate-orm-guide

@sebastienblanc
Copy link
Contributor

     11. Show the code for the client side in the guides

Instead of showing curl commands ? That would be nice but also will be a bit out of scope of Quarkus, and what frontend techno will you show : vanilla JS, Angular, Vue, React ?

I mean a Quarkus REST client talking to a secured service.

Ah I see, and yes that would be great and with Quarkus it's so easy, in my example I'm using the rest client in combination with the propagateHeaders feature : https://github.com/sebastienblanc/quarkus-quickstart/blob/master/quarkus-rest-username/src/main/resources/application.properties#L3

So you have nothing to do beside calling your secured service.

@sebastienblanc
Copy link
Contributor

I do understand that it's not the Keycloak nor the JWT extension core feature to generate a JWT token.
I was just exploring 1. why it is described in the guide if you don't need to 2. how to test things either as one shot or in a automated test. So I was thinking that the keycloak extension could simulate a JWT token and pass it without needing a server. Maybe that's not useful but that was my train of thought

For 1. I think it's still relevant to have it. People might want to try out quickly the jwt extension and they don't want to set up a whole identity Server.
For 2. I think Nimbus is for now enough to write tests.

@sberyozkin
Copy link
Member

There is a request to enhance MP JWT API with the portable builder API to get the tokens self-issued/generated, can be useful for the tests. The only dilemma is whether to make that part of MP-JWT or of something independent.
As for combining Keycloak and MP-JWT, if we can make the tokens validated by the KC adapter later accessible from the user code via MP-JWT then we can say the combination is working. We did it in Thorntail

@sebastienblanc
Copy link
Contributor

There is a request to enhance MP JWT API with the portable builder API to get the tokens self-issued/generated, can be useful for the tests. The only dilemma is whether to make that part of MP-JWT or of something independent.
As for combining Keycloak and MP-JWT, if we can make the tokens validated by the KC adapter later accessible from the user code via MP-JWT then we can say the combination is working. We did it in Thorntail

You mean being able to inject JsonWebtoken and also use @Claim when using the keycloak extension ? That would be awseome.

@sberyozkin
Copy link
Member

sberyozkin commented Apr 26, 2019

@sebastienblanc yes, exactly. But I'm not sure yet about the path in Quarkus, in Thorntail we bypass the actual KC adapter and use a light-weight smallrye-jwt KC factory. May be something similar can be done in Quarkus smallrye-jwt or as @pedroigor suggested, interposing JsonWebToken principal on top of the one prepared by the KC adapter will do the trick. I'm positive it can be done somehow :-)

@starksm64
Copy link
Contributor

We should look at having the smallrye-jwt extension be able to perform the JsonWebToken and @claim injection whenever there is a MP-JWT compatible bearer token source. The keycloak extension should just enable this when the adaptor is configured with that type of bearer token.

The thing that needs to be reconciled is the authenticated user account representation that the JsonWebToken and @claim injection is based on. I'll write up a design discussion doc and try to schedule a call on this issue next week.

@pedroigor
Copy link
Contributor

pedroigor commented Apr 29, 2019

I agree with most of the items in that list, mainly those related with clarifications to documentation and the alignment between Keycloak extension and MP-JWT API/extension.

I've looked micronaut-security-jwt and the fact that you encourage self-issued tokens is not a good idea. It seems they are kind of trying to force some OAuth2 alignment (see the response and the idea of refresh tokens) where there is much more to be considered when a service also becomes a token authority. IMO, this encourages a bad practice, not very flexible and force developers to worry about security considerations that are already part of any OIDC/OAuth compliant server such as Keycloak.

I like the idea of mocking tokens and using them during tests. However, I'm not sure how this impacts coverage considering that you may have additional settings and capabilities involved when issuing and consuming these tokens. For instance, if your application relies on some social provider authentication, you won't be testing that. Or if you have specific settings in Keycloak that you want to cover when writing a test for a protected service. In addition to that, docker makes things easier so that you can easily write a fully functional and integrated test. Self-issued tokens may be nice for a demo, but not sure if this holds true for a real deployment.

In regards to OIDC/OAuth and JWT, we should keep these concepts separated. Being the most common approach for token-based authorization, OAuth does not mention JWT. Neither JWT is related to OAuth. However, the OAuth-WG is working on a JWT profile for OAuth access tokens that maybe we could consider in the MP-JWT extension.

The Keycloak extension is really targeted for deployments using Keycloak(for OIDC/OAuth), it should not be considered a general OIDC/OAuth/JWT implementation. But yeah, we could make it more MP-JWT compliant for sure ...

@emmanuelbernard
Copy link
Member Author

Hey all,
This is a recurring feedback we hear when at conference. We really need to improve on the guides and merge them. Who could drive a v1 home on that part at least? @sebastienblanc I know at some point you volunteered, can you still try?

@emmanuelbernard
Copy link
Member Author

The other feedback is that people tried to set up OAuth with our doc and failed.

@justintime4tea
Copy link

I was able to get the "using-keycloak" set up fairly easy but I cannot for the life of my get one single endpoint to be @permitAll (anonymous). It seems I either need to disable enforcement (permissive does not work) or make every endpoint require login if any of them need login. Basic vanilla "using-keycloak" quickstart and using application.properties to configure Keycloak.

In context: I've had great success working on the Keycloak core project as well as a 3rd party module for it so this shouldn't be such a difficult task to accomplish. Feeling like I've exhausted resources and don't know where to look next. I get that Quarkus is incredibly new so I am looking forward to better docs, better examples/quickstarts, better integration and more maturity because I am truly excited for Quarkus.

@emmanuelbernard
Copy link
Member Author

Also see current proposal on OAuth2 (PR) #3078

@gsmet
Copy link
Member

gsmet commented Nov 13, 2019

@sberyozkin @pedroigor @sebastienblanc so what's the status of this? Should it be closed? Is there still some work required? If so, what?

@stefnotch
Copy link
Contributor

stefnotch commented Mar 10, 2020

One still outstanding issue is that this tutorial says

The /subject/unsecured endpoint allows for unauthenticated access by specifying the @PermitAll annotation.

This does seem to imply that anonymous access should work with the @PermitAll annotation

However, in practice, this doesn't seems to work for anonymous access

Another relevant issue regarding anonymous access is #6807

@sberyozkin
Copy link
Member

sberyozkin commented Apr 2, 2020

@stefnotch thanks for these links, but it is not a security doc bug, I'm not sure we need to start deleting the doc texts everytime an issue is open which points to some inconsistency with the docs and the actual quarkus behaviour :-).
We are discussing with @pedroigor how to deal with #6807, hopefully it will be addressed soon enough.

Let me close this issue then, it would be easier to keep talking about the specific issues in the dedicated issues like #6807 thanks

@gsmet gsmet added the triage/out-of-date This issue/PR is no longer valid or relevant label Apr 7, 2020
@MarcusBiel
Copy link

MarcusBiel commented Jun 28, 2020

I see this issue has been closed a few months ago... but have the issues raised actually been addressed so far? I am and or have been struggling with the issues described in this thread - lots of different tutorial resources on the topic, also I found @permitAll confusing, and it took me long to find a way to get enable a full unauthorized endpoint (for a stripe webhook) - since I have globally protected all endpoints on /api/ as described in one of those tutorials. Finally, I would love to see some examples on integration testing oauth2 secured endpoints. In Micronaut security I did not run into as much troubles, to be honest.

@emmanuelbernard
Copy link
Member Author

Let's open dedicated issues to the problem you have experienced. We will be able to hunt them down more easily. PS that's also a reason why I prefer more concrete actionable titles for issues like that. Improve foo is a never ending task ;)

@MarcusBiel
Copy link

MarcusBiel commented Jun 29, 2020 via email

@emmanuelbernard
Copy link
Member Author

emmanuelbernard commented Jun 30, 2020 via email

@MarcusBiel
Copy link

Done.
#10361
#10362
#10363

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security triage/out-of-date This issue/PR is no longer valid or relevant
Projects
None yet
Development

No branches or pull requests

10 participants