quarkusio / quarkus

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

SecurityIdentity not being set in websocket OnOpen, OnMessage, etc callbacks #16847

Open Sboddd opened 3 years ago

Sboddd commented 3 years ago

Describe the bug

I have a Quarkus (1.6.1.Final) application that uses quarkus-oidc for user authentication. My server has a websocket endpoint:

@ApplicationScoped @ServerEndpoint(value="/websocket") public class WebsocketEndpoint { @Inject SecurityIdentity identity;

@OnOpen public void onOpen(Session session) { // at this point identity is always Anonymous, even if I use a valid auth header that works correctly on a normal REST endpoint. } } I'd like to be able to do some user authentication on this endpoint, ideally via a @RolesAllowed annotation on the class. From hooking up a debugger, I can step through the OidcAuthenticationMechanism and validate that a SecurityIdentity object is being constructed and correctly reflects the contents of my JWT - but, by the time I get to my OnOpen callback, it's no longer set. (Likewise, any attempt to use a @RolesAllowed on my endpoint fails because the SecurityIdentity is an anonymous user in the RolesAllowedCheck call.) The same JWT yields a correctly populated SecurityIdentity when used to access a REST endpoint.

Expected behavior

SecurityIdentity should be populated in websocket callbacks.

Actual behavior

SecurityIdentity is an anonymous user in websocket callbacks.

To Reproduce

I haven't had a chance to make a full reproducer yet. My application is using Quarkus 1.6.1.Final, non-native, Gradle build.

Steps to reproduce:

  1. Create a Quarkus application using quarkus-oidc with a websocket endpoint per the example above.
  2. Have a client application acquire a JWT and connect to the websocket.
  3. In the OnOpen callback, log or inspect via debugger the injected SecurityIdentity; observe that it reflects an anonymous user.

Configuration

# Possibly relevant excerpts from our properties file:
quarkus.websocket.max-frame-size=2621440
quarkus.websocket.dispatch-to-worker=true
quarkus.oidc.auth-server-url=${OAUTH_OIDC_URL}
quarkus.oidc.credentials.secret=${OAUTH_CLIENT_SECRET}
quarkus.oidc.client-id=${OAUTH_CLIENT_ID}

Screenshots

n/a

Environment (please complete the following information):

Output of uname -a or ver

Linux myhostname 3.10.0-1062.12.1.el7.x86_64 #1 SMP Thu Dec 12 06:44:49 EST 2019 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.9.1" 2020-11-04 LTS OpenJDK Runtime Environment 18.9 (build 11.0.9.1+1-LTS) OpenJDK 64-Bit Server VM 18.9 (build 11.0.9.1+1-LTS, mixed mode, sharing)

GraalVM version (if different from Java)

Same.

Quarkus version or git rev

1.6.1.Final

Build tool (ie. output of mvnw --version or gradlew --version)


Gradle 6.5.1

Build time: 2020-06-30 06:32:47 UTC Revision: 66bc713f7169626a7f0134bf452abde51550ea0a

Kotlin: 1.3.72 Groovy: 2.5.11 Ant: Apache Ant(TM) version 1.10.7 compiled on September 1 2019 JVM: 11.0.5 (AdoptOpenJDK 11.0.5+10) OS: Windows 10 10.0 amd64

Additional context

(Add any other context about the problem here.)

quarkus-bot[bot] commented 3 years ago

/cc @evanchooly, @sberyozkin

sberyozkin commented 3 years ago

@Sboddd Hi, I don't think it is possible, we've had a few issues before - I think you need to come up with a custom approach to make sure the token originally available in the normal REST request is made available to Quarkus - perhaps by using a custom web socket binding etc. I'm going to close this issue - you can reopen if you'd like - but I think this response from a user on the Zulip channel is good. CC @stuartwdouglas

Sboddd commented 3 years ago

Thanks for the response! Is that something that's worth adding to the Quarkus OIDC documentation (e.g. here: https://quarkus.io/guides/security-openid-connect), possibly even with an example workaround?

stuartwdouglas commented 3 years ago

This is definitely a valid feature request, it will just take a bit of work to implement on the WebSocket side.

sberyozkin commented 3 years ago

Hi Stuart, I can imagine how it can work in onOpen, but what about onMessage ? Do you have a dedicated binding in mind or something else ? thanks

stuartwdouglas commented 3 years ago

You just attach the identity to the connection and setup the association before calling the method.

sberyozkin commented 3 years ago

@stuartwdouglas - thanks - do you mean attach the identity obtained during OnOpen and then re-use it whenever a callback happens with OnMessage ? Most likely I'm missing something - but then there is a risk of the identity going stale - for example, the user may have signed off from OIDC/etc. If it is indeed a possible risk then a custom binding might work - but I guess only for the tokens (with quarkus-oid/smallrye-jwt) as opposed to say basic auth.

stuartwdouglas commented 3 years ago

If you are worried about that then you need to close the websocket connection on logout.

sberyozkin commented 3 years ago

Sure but it becomes a user responsibility. In a way we have something similar with the quarkus-oidc code flow by using cookies to keep the session alive - but at least those cookies are time scoped - yeah, I guess if the attached identity is also time scoped then it should be fine - if the identity was created from the OIDC token then we can pass the max age of the session as a SecurityIdentity attribute for the web sockets code to take that into consideration... in other cases the max age of this attached identity can be configurable, etc...

ethan-gallant commented 3 years ago

When writing an implementation of GraphQL subscriptions over WebSocket I have something similar to the above. We used OIDC and created an HttpAuthenticationMechanism that delegates to the OidcAuthenticationMechanism.

The client performs something similar to the code flow, however, it sends the code in the query params. The HttpAuthenticationMechanism picks up that it is a WebSocket request and populates a SecurityIdentity with the refresh and access token.

Whenever a new request is made over the WebSocket, the application checks if the token is stale and will attempt to refresh the Security Identity with the refresh token credential attached. Not sure if this is the best solution but it stops the problem of tokens going stale and it also avoids exposing an access token in logging due to sensitive parameters being passed in the url/query.

aldoborrero commented 3 years ago

I can confirm as well that even using a basic implementation from HttpAuthenticationMechanism does not populate correctly SecurityIdentity on websocket methods. I do see value as well on having the proper SecurityIdentity populated and accessible over websockets.

In our particular case, we can check in onMessage if the token is stale or not and take an action depending on the result (which is closing the connection).