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

quarkus-websockets-next connecting from browser with JWT #42824

Closed
rasmushaglund opened this issue Aug 28, 2024 · 18 comments · Fixed by #45809
Closed

quarkus-websockets-next connecting from browser with JWT #42824

rasmushaglund opened this issue Aug 28, 2024 · 18 comments · Fixed by #45809

Comments

@rasmushaglund
Copy link

rasmushaglund commented Aug 28, 2024

Description

From what I understand the RFC6455 WebSockets specification allows for token-based auth using request headers, but in the client Javascript WebSocket implementation it's not possible to send authorization headers.

I have a setup with using quarkus-websockets-next together with KeyCloak and would need to authenticate using the token retrieved by KeyCloak via a Javascript WebSocket.

Implementation ideas

I've seen multiple suggestions on how this limitation is bypassed but most of them seem to involve setting the token in the url, for example:
ws://${token}@localhost/ws or ws://localhost/ws?token=${token}.

Is there a way to solve this today, or is this something that would need to be added to the extension?

Thank you, and thank you for all the good work you do on this project, it's much appreciated!

@rasmushaglund rasmushaglund added the kind/enhancement New feature or request label Aug 28, 2024
Copy link

quarkus-bot bot commented Aug 28, 2024

/cc @sberyozkin (jwt)

@quarkusio quarkusio deleted a comment Aug 28, 2024
@sberyozkin
Copy link
Member

@rasmushaglund Hi, my understanding that to make it work from Java Script, you'd need a subprotocol header to get the token available at the upgrade time stick around, I'm planning to work on a demo as part of the quarkus-langchain4j project to clarify some of these details for myself.
CC @mkouba, AFAIK, it is not really in scope for websockets-next as such, users need to customize the protocol to get the token flowing, right ? I've seen a few example over the last couple of years but never properly focused, I'll need to focus on it asap...

@rasmushaglund
Copy link
Author

Thank you @sberyozkin! Yeah, at least it would be super helpful with some simple example for how this can be done, since I guess that it might be a pretty common use case to use Websockets with authentication through Javascript.

I have very limiting knowledge of how Websockets work, but I'll give it a shot based on your explanation.

Thanks!

@mkouba
Copy link
Contributor

mkouba commented Aug 28, 2024

AFAIK, it is not really in scope for websockets-next as such, users need to customize the protocol to get the token flowing, right ? I've seen a few example over the last couple of years but never properly focused, I'll need to focus on it asap...

I think that I saw workarounds using the query params and even abusing the Sec-WebSocket-Protocol header. Both can be inspected in a server endpoint callback method using the HandshakeRequest. You will likely need to modify the headers of the HTTP upgrade request. The io.quarkus.websockets.next.HttpUpgradeCheck API may come in handy.

I'm not sure what the correct solution is 🤷.

CC @cescoffier

@sberyozkin
Copy link
Member

Thanks @mkouba, I'll have to figure it all out with quarkiverse/quarkus-langchain4j#609 :-)

@Niavana97
Copy link

I use "token" URL parameters and HttpUpgradeCheck to verify user identity, but I don't know how to add the user to the SecurityContext.

@indiealexh
Copy link

The HttpUpgradeCheck seems a little too late, as a securityIdentity is already set in the context.

I don't know what works or not, but https://quarkus.io/guides/security-customization#dealing-with-more-than-one-http-auth-mechanisms is where I am going to try.

@gsmet
Copy link
Member

gsmet commented Jan 9, 2025

If we end up with working recommendations, let's make sure we document them in the guide. Thanks!

@michalvavrik
Copy link
Member

michalvavrik commented Jan 9, 2025

If we end up with working recommendations, let's make sure we document them in the guide. Thanks!

Personally I'd not document this because using URL is not a good solution.

Honestly, I am getting lost in opened / closed issues and Zulip chats, but this has been discussed many times already. For example here is one such opened issue #44409. If you insist on using https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API/Writing_WebSocket_client_applications then indeed your options are limited. When I was researching possible solutions, I saw alternative JS clients that allowed to send headers with initial handshake, but I didn't have time to evaluate whichone I would actually trust in production. ATM my opinion is that the best option is to use headers and then we should implement #43434. It is on my list, but I need people to comment there because I am not sure whether my proposal is the right way to do it.

@michalvavrik
Copy link
Member

If you were just saying - let's document it once it is solved or there is a consensus - absolutely +1

@michalvavrik
Copy link
Member

michalvavrik commented Jan 9, 2025

The HttpUpgradeCheck seems a little too late, as a securityIdentity is already set in the context.

I don't know what works or not, but https://quarkus.io/guides/security-customization#dealing-with-more-than-one-http-auth-mechanisms is where I am going to try.

Technically it is not too late if you do not require the identity during HTTP upgrade check and if you don't use security annotations to secure the upgrade, for example if you inject the identity and check it manually. But the HTTP authentication mechanism is the right way because this is not specific to WS Next and the fact you can se HTTP upgrade check for certain scenarios is just an implementation detail.

@michalvavrik
Copy link
Member

michalvavrik commented Jan 9, 2025

also, looking at the https://quarkus.io/guides/websockets-next-reference#websocket-next-security I think more documentation is required because we have it all tested, but I should had written examples from our tests. I'll take care of it.

@indiealexh
Copy link

Honestly, I just wish I could go fix the Browser JavaScript spec.
Everything else is just a hacky workaround.

...not too late if you do not require the identity...

Unfortunately I need access to the user and roles for my use case.

If I find a nice solution, I'll document it and prep a pr.

@michalvavrik
Copy link
Member

Unfortunately I need access to the user and roles for my use case.

just to be sure we understand each other, I meant:

  • you can use the upgrade check if you need user and roles in your endpoint or security annotations security ..OnMessage.. method
  • HTTP mechanism is a way to go if you need to extract credentials manually, I never suggested the upgrade check, just provide my POV on your comment in case it was useful
  • we do support combination of WS-NEXT and OIDC with headers and have it tested, so you don't need the HTTP auth mechanism if you are able to remap a token from "xyz" to headers using Vert.x route handler (because I think these maps are mutable)

If I find a nice solution, I'll document it and prep a pr.

cool, please have a look at the issues I linked as well for more context

Personally, I would prefer builtin solution, not documenting creation of custom HTTP auth mechanism (it is already documented for it is same for any HTTP request). There is plenty of users asking for this. If you find a solution, I'll be happy to hear about it. Thanks

@michalvavrik
Copy link
Member

I think that I saw workarounds using the query params and even abusing the Sec-WebSocket-Protocol header. Both can be inspected in a server endpoint callback method using the HandshakeRequest. You will likely need to modify the headers of the HTTP upgrade request. The io.quarkus.websockets.next.HttpUpgradeCheck API may come in handy.

As for even abusing the Sec-WebSocket-Protocol header https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API/Writing_WebSocket_client_applications#protocols I think that is probably the safest solution and it works plain JS WS client. I don't think users most users know how to do that so that this header is later used by Quarkus OIDC. @sberyozkin @mkouba should we provide builtin solution that abuse this header when a some configuration property is set to true or is it no go?

@michalvavrik
Copy link
Member

it doesn't make it right, but I just found that K8 does it too in some form https://github.com/kubernetes/kubernetes/blob/master/pkg/kubeapiserver/authenticator/config.go#L198. I'll have to go deeper because I don't know context yet.

@mkouba
Copy link
Contributor

mkouba commented Jan 22, 2025

should we provide builtin solution that abuse this header when a some configuration property is set to true or is it no go?

@michalvavrik I'm not so sure to be honest. I'd probably prefer to document the workaround and examples.

@michalvavrik
Copy link
Member

should we provide builtin solution that abuse this header when a some configuration property is set to true or is it no go?

@michalvavrik I'm not so sure to be honest. I'd probably prefer to document the workaround and examples.

ok, I'll do that then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
@sberyozkin @rasmushaglund @mkouba @indiealexh @gsmet @Niavana97 @michalvavrik and others