quarkusio / quarkus

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

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

Closed emmanuelbernard closed 4 years ago

emmanuelbernard commented 5 years ago

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:

stephanj commented 5 years ago

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 commented 5 years ago
  • [ ] 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 :

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 commented 5 years ago
     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 commented 5 years ago
  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 commented 5 years ago
     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 commented 5 years ago
     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 commented 5 years ago
     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 commented 5 years ago
     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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

justintime4tea commented 5 years ago

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 commented 5 years ago

Also see current proposal on OAuth2 (PR) https://github.com/quarkusio/quarkus/pull/3078

gsmet commented 4 years ago

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

stefnotch commented 4 years ago

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 https://github.com/quarkusio/quarkus/issues/6807

sberyozkin commented 4 years ago

@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

MarcusBiel commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Sure. Who. Where. How.

Am 29. Juni 2020, 13:56, um 13:56, Emmanuel Bernard notifications@github.com schrieb:

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 ;)

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/quarkusio/quarkus/issues/2231#issuecomment-651068350

emmanuelbernard commented 4 years ago

On Mon 29 Jun 2020 at 23:31, Marcus Biel notifications@github.com wrote:

Sure. Who.

You :) you would be the best to capture the need.

Where.

quarkusio/Quarkus GitHub issue is where we capture everything.

How.

Add the area/security label and @mention Sergey and I.

MarcusBiel commented 4 years ago

Done. https://github.com/quarkusio/quarkus/issues/10361 https://github.com/quarkusio/quarkus/issues/10362 https://github.com/quarkusio/quarkus/issues/10363