quarkusio / quarkus

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

Allow security passthrough #2290

Open jtama opened 5 years ago

jtama commented 5 years ago

Actually I don't know of it's a documentation issue, an extension issue or a feature request.

When using smallrye-health in combination with keycloak extension, health check resource become secured, which is certainly not the desired behaviour.

Keycloak extension does not provide a mechanisme I know of to permit all trafic for a given path, and PermitAll annotation can obviously not be applied on the Health servlet.

Please let me know if I am missing something.

I am using following dependencies management :

<dependencies>
       <dependency>
           <groupId>io.quarkus</groupId>
           <artifactId>quarkus-bom</artifactId>
           <version>0.14.0</version>
           <type>pom</type>
           <scope>import</scope>
       </dependency>
</dependencies>
jtama commented 5 years ago

Solution found : Just created a dedicated conf for /health path in the keycloak configuration file :

{
  "realm":"App",
  "auth-server-url":"http://localhost:8080/auth",
  "resource":"Quarkus",
  "bearer-only": true,
  "credentials": {
    "secret": "secret"
  },
  "policy-enforcer" : {
    "enforcement-mode":"PERMISSIVE",
    "paths": [
      {
        "name": "unsecured",
        "path": "/health",
        "enforcement-mode" : "DISABLED"
      }
    ]
  }
}
gsmet commented 5 years ago

@starksm64 is there something we can improve from a user experience point of view? I don't know if it can be done automatically?

Not sure it's entirely secure either as your health checks might contain sensitive information and you might want to secure them somehow.

Interested in your feedback.

jtama commented 5 years ago

Hi, Health check is certainly to be used by your Container orchestrator, and should not be secured (I think).

It may be that documenting the multiples security related extensions is enough.

As it works now, Keycloak extension security handler has high priority in the server filter chain, and the health servlet is placed way down in the stack.

From my understanding, if security pass through is to be handled by the smallrye-health extension, it should add a very high priority handler that skip other handler in the chain for dedicated path.

starksm64 commented 5 years ago

I'm including this issue in the security arch discussion doc I'm putting together to address our inconsistencies

NilsWild commented 5 years ago

@jtama-op did you change anything in the keycloak configuration of the quarkus keycloak starter project? For me the enforcement-mode: DISABLED doesn't work at all. Usualy PERMISSIVE should already be enought as there is no rule defined to match /health so it should be allowed.,not sure though what to change

also see: https://github.com/quarkusio/quarkus/issues/2231#issuecomment-507120194

jtama commented 5 years ago

I did not used the keycloak starter. Please find my configuration bellow (i switched to application.properties).

quarkus.keycloak.auth-server-url = http://${KEYCLOAK_HOST:localhost}:8080/auth
quarkus.keycloak.resource = Quarkus
quarkus.keycloak.bearer-only: false
quarkus.keycloak.credentials.secret: ${KEYCLOAK_SECRET:secret}
quarkus.keycloak.policy-enforcer.enforcement-mode = PERMISSIVE
quarkus.keycloak.policy-enforcer.paths.health.name = Heath
quarkus.keycloak.policy-enforcer.paths.health.path = /health
quarkus.keycloak.policy-enforcer.paths.health.enforcement-mode = DISABLED
quarkus.keycloak.policy-enforcer.paths.OpenAPEnuI.name = OpenAPI
quarkus.keycloak.policy-enforcer.paths.OpenAPI.path = /openapi
quarkus.keycloak.policy-enforcer.paths.OpenAPI.enforcement-mode = DISABLED

It may be that the quarkus.keycloak.policy-enforcer.enforcement-mode = PERMISSIVE isn't usefull. I haven't test removing it.

Hope it helps.

NilsWild commented 5 years ago

@jtama-op Thanks but as you didn't set quarkus.keycloak.policy-enforcer.enable = true the keycloak PEP isn't used at all. That way you don't have any authz at all as that option is false by default. I either have full auth or none at all.

jtama commented 5 years ago

Ok, Sounds weird, I will re-check and come back to you.

jtama commented 5 years ago

Ok so i got my desired behaviour back.

Please find steps to reproduce :

  1. Create dedicated realm : quarkus
  2. Create client for this realm : backend-service
  3. Set access type to confidential
  4. Save
  5. Retrieve client secret 1 Configure application as bellow :
    quarkus.keycloak.realm=quarkus
    quarkus.keycloak.auth-server-url=http://<keycloak-url>/auth
    quarkus.keycloak.resource=backend-service
    quarkus.keycloak.bearer-only=false
    quarkus.keycloak.credentials.secret: <secret>
    quarkus.keycloak.policy-enforcer.enable=true
    quarkus.keycloak.policy-enforcer.enforcement-mode=PERMISSIVE
    quarkus.keycloak.policy-enforcer.paths.health.name=Heath
    quarkus.keycloak.policy-enforcer.paths.health.path=/health
    quarkus.keycloak.policy-enforcer.paths.health.enforcement-mode=DISABLED
    quarkus.keycloak.policy-enforcer.paths.OpenAPI.name=OpenAPI
    quarkus.keycloak.policy-enforcer.paths.OpenAPI.path=/openapi
    quarkus.keycloak.policy-enforcer.paths.OpenAPI.enforcement-mode=DISABLED

Health check and swagger-ui will be enabled, other resources will be secured.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you! We are doing this automatically to ensure out-of-date issues does not stay around indefinitely. If you believe this issue is still relevant please put a comment on it on why and if it truly needs to stay request or add 'pinned' label.

gsmet commented 4 years ago

I think we at least need to improve the documentation and probably think a bit more about how to deal with health and security. We already had some discussion about that.

gsmet commented 4 years ago

/cc @sberyozkin

jtama commented 4 years ago

I think we should definitely close this issue, as I also think we should not use keycloak extension...

gastaldi commented 8 months ago

What's the status of this issue? Should we close it as is? /cc @gsmet