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

Allow security passthrough #2290

Open
jtama opened this issue Apr 30, 2019 · 14 comments
Open

Allow security passthrough #2290

jtama opened this issue Apr 30, 2019 · 14 comments
Labels
pinned Issue will never be marked as stale

Comments

@jtama
Copy link
Contributor

jtama commented Apr 30, 2019

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
Copy link
Contributor Author

jtama commented May 2, 2019

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"
      }
    ]
  }
}

@jtama jtama closed this as completed May 2, 2019
@gsmet gsmet reopened this May 2, 2019
@gsmet
Copy link
Member

gsmet commented May 2, 2019

@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
Copy link
Contributor Author

jtama commented May 3, 2019

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
Copy link
Contributor

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

@NilsWild
Copy link

NilsWild commented Jul 16, 2019

@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: #2231 (comment)

@jtama
Copy link
Contributor Author

jtama commented Jul 17, 2019

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
Copy link

@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
Copy link
Contributor Author

jtama commented Jul 17, 2019

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

@jtama
Copy link
Contributor Author

jtama commented Jul 19, 2019

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
Copy link

stale bot commented Nov 13, 2019

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.

@stale stale bot added the stale label Nov 13, 2019
@maxandersen maxandersen removed the stale label Nov 13, 2019
@gsmet gsmet added the pinned Issue will never be marked as stale label Nov 13, 2019
@gsmet
Copy link
Member

gsmet commented Nov 13, 2019

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
Copy link
Member

gsmet commented Nov 13, 2019

/cc @sberyozkin

@jtama
Copy link
Contributor Author

jtama commented Dec 23, 2019

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

@gastaldi
Copy link
Contributor

gastaldi commented Jan 16, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Issue will never be marked as stale
Projects
None yet
Development

No branches or pull requests

6 participants