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

WebSocket auth on VertxCurrentContext not working #29919

Closed
RoMiRoSSaN opened this issue Dec 16, 2022 · 14 comments · Fixed by #29950
Closed

WebSocket auth on VertxCurrentContext not working #29919

RoMiRoSSaN opened this issue Dec 16, 2022 · 14 comments · Fixed by #29950

Comments

@RoMiRoSSaN
Copy link

RoMiRoSSaN commented Dec 16, 2022

Describe the bug

Hello. In my comman we use auth on WebSocket class by this hack

@RouteFilter(401)
void addAuthHeader(RoutingContext rc) {
    try {
        if (rc.request().headers().get("Sec-WebSocket-Protocol") != null) {
            String token = "Bearer " + rc.request().headers().get("Sec-WebSocket-Protocol").split(", ")[1];
            rc.request().headers().add("Authorization", token);
        }
    } finally {
        rc.next();
    }
}

We make ws request such this

const ws = new WebSocket(`ws://localhost:8080/ws`, ["access_token", "token"]);

And WebSoket class such as

@ServerEndpoint(
        value = "/api/v1/notification-socket/{userId}",
        decoders = SocketMessageDecoder.class,
        encoders = SocketMessageEncoder.class
)
@Authenticated
public class WS {
    // ......
}

Last version when it works is 2.12.3.Final
It works by changes in this commit - Add proper identity propagation to WebSockets - File WebsocketCoreRecorder

After update to version 2.13 and higher it work only then used property
quarkus.vertx.customize-arc-context=false
In version 2.13 added changes from this commit CDI context propagation improvements for the reactive stack.
Now where is two CurrentContext implementations - ThreadLocalCurrentContext (ws auth works) and VertxCurrentContext (ws auth not work)

This code from WebsocketCoreRecorder

boolean required = !requestContext.isActive();
if (required) {
    requestContext.activate();
    Principal p = session.getUserPrincipal();
    if (p instanceof WebSocketPrincipal) {
        CurrentIdentityAssociation current = this.getCurrentIdentityAssociation();
        if (current != null) {
            current.setIdentity(((WebSocketPrincipal)p).getSecurityIdentity());
        }
    }
}

When uses ThreadLocalCurrentContext - requestContext.isActive() is false, and principal added in context
When uses VertxCurrentContext - requestContext.isActive() is always true, and user don`t added on context

Maybe fix this place, or create fix VertxCurrentContext?
Thank you for your job)

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.13+

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

No response

Additional information

No response

@sberyozkin
Copy link
Member

Update the link to the #27443 PR. Hi Martin, @mkouba, have a look please when you get a chance

@mkouba
Copy link
Contributor

mkouba commented Dec 16, 2022

Hm, I'm no websockets expert but I will try to look at the code in the WebsocketCoreRecorder.

@mkouba
Copy link
Contributor

mkouba commented Dec 16, 2022

I might be wrong but I think that the current identity should be set even if the CDI request context is already active, or?

@sberyozkin @RoMiRoSSaN it would be great if you could test the fix in this branch: https://github.com/mkouba/quarkus/tree/issue-29919 (i.e. checkout the branch, build the project and run your app/reproducer)

@sberyozkin
Copy link
Member

Thanks @mkouba :-), @RoMiRoSSaN can you check if the proposed fix addresses the problem ?

@RoMiRoSSaN
Copy link
Author

RoMiRoSSaN commented Dec 19, 2022

@mkouba @sberyozkin Hello. I tested it localy. It works correct

Only when WS class annotated @authenticated and JS ws request not contains token - throws unauthorized exeprtion

Caused by: io.quarkus.security.UnauthorizedException
	at io.quarkus.security.runtime.interceptor.check.AuthenticatedCheck.doApply(AuthenticatedCheck.java:38)
	at io.quarkus.security.runtime.interceptor.check.AuthenticatedCheck.apply(AuthenticatedCheck.java:25)
	at io.quarkus.security.runtime.interceptor.SecurityConstrainer.check(SecurityConstrainer.java:32)
	at io.quarkus.security.runtime.interceptor.SecurityHandler.handle(SecurityHandler.java:46)
	at io.quarkus.security.runtime.interceptor.AuthenticatedInterceptor.intercept(AuthenticatedInterceptor.java:29)
	at io.quarkus.security.runtime.interceptor.AuthenticatedInterceptor_Bean.intercept(Unknown Source)
	at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
	at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:41)
	at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:33)
	at com.example.StartWebSocket_Subclass.onError(Unknown Source)
	at jdk.internal.reflect.GeneratedMethodAccessor40.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at io.undertow.websockets.annotated.BoundMethod.invoke(BoundMethod.java:87)
	at io.undertow.websockets.annotated.AnnotatedEndpoint$5.run(AnnotatedEndpoint.java:225)
	at io.undertow.websockets.ServerWebSocketContainer$1.call(ServerWebSocketContainer.java:143)
	at io.undertow.websockets.ServerWebSocketContainer$1.call(ServerWebSocketContainer.java:140)
	at io.quarkus.websockets.client.runtime.WebsocketCoreRecorder$4$1.call(WebsocketCoreRecorder.java:181)
	at io.undertow.websockets.ServerWebSocketContainer.invokeEndpointMethod(ServerWebSocketContainer.java:532)

But I think if you need ws authorize you will send token always) and this exception is not error)
I can create example app for you for test if you want

Thanks!)

@RoMiRoSSaN
Copy link
Author

For other peaople who need auth in WS

Create reactive route

@Singleton
public class WSAuthFilter {
    @RouteFilter(401)
    void addAuthHeader(RoutingContext rc) {
        try {
            if (rc.request().headers().get("Sec-WebSocket-Protocol") != null) {
                String token = "Bearer " + rc.request().headers().get("Sec-WebSocket-Protocol").split(", ")[1];
                rc.request().headers().add("Authorization", token);
            }
        } finally {
            rc.next();
        }
    }
}

Create custom ServerEndpointConfig.Configurator

public class WebSocketSecurityConfigurator extends ServerEndpointConfig.Configurator {
    @Override
    public void modifyHandshake(ServerEndpointConfig sec, HandshakeRequest request, HandshakeResponse response) {
        try {
            String subProtocol = request.getHeaders().get("Sec-WebSocket-Protocol").get(0).split(",")[0];
            response.getHeaders().put("sec-websocket-protocol", List.of(subProtocol));
        } catch (Throwable ignore) {
        }
    }
}

Create WS class

@Authenticated
@ServerEndpoint(
        value ="/ws",
        configurator = WebSocketSecurityConfigurator.class
)
public class WebSocket {
    @Inject
    UserInfo userInfo;
    // .....
}

In JS

const ws = new WebSocket(`ws://localhost:8080/ws`, ["access_token","your token"] );

@mkouba
Copy link
Contributor

mkouba commented Dec 19, 2022

Only when WS class annotated https://github.com/authenticated and JS ws request not contains token - throws unauthorized exeprtion

@sberyozkin is it ok? If so I can send a pull request.

@mkouba
Copy link
Contributor

mkouba commented Dec 19, 2022

Hello. I tested it localy. It works correct

Thanks @RoMiRoSSaN!

@sberyozkin
Copy link
Member

Thanks @mkouba for this fix.
@RoMiRoSSaN Would you like to open a PR and update https://quarkus.io/guides/websockets ? (docs/src/main/asciidoc/websockets.adoc).
For example, add a section Authentication and start with something like, If you would like to use OpenId Connect tokens to have WebSocket requests authenticated then you can do it like this: and show 3 steps you have suggested above.
It would be useful as well to expand https://github.com/quarkusio/quarkus/blob/main/integration-tests/oidc/src/test/java/io/quarkus/it/keycloak/WebsocketOidcTestCase.java with an HtmlUnit test to make sure no side-effects are introduced in the future

@gsmet gsmet modified the milestones: 2.16 - main, 2.15.1.Final Dec 20, 2022
@flowt-au
Copy link

@RoMiRoSSaN Thanks your your code fragments. I spent hours trying to find out how to do this before stumbling on your post above.

I am having trouble getting it to run because, I think, I might be using the incorrect classes (eg for ServerEndpointConfig, etc). I am fairly new to Java so I am not sure which Maven dependencies I should include.

Would you be able to post the pom dependencies and imports please so I can be sure I am using the correct libraries?

Are these the correct ones?

<dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-reactive-routes</artifactId>
</dependency>
import io.quarkus.vertx.web.RouteFilter;
import io.vertx.ext.web.RoutingContext;

import javax.websocket.HandshakeResponse;
import javax.websocket.server.HandshakeRequest;
import javax.websocket.server.ServerEndpointConfig;

I am currently getting

Unable to instantiate endpoint: java.lang.RuntimeException: java.lang.IllegalStateException: The current thread cannot be blocked: vert.x-eventloop-thread-0
2023-01-16 18:27:51     at io.undertow.websockets.ServerWebSocketContainer.invokeEndpointMethod(ServerWebSocketContainer.java:534)

which to me looks like I am using the wrong library, I think.

I thought Quarkus used Jakarta Websockets. You can see I am very confused!

Thanks,
Much appreciated,
Murray

@flowt-au
Copy link

I just thought to say I was testing first without sending the token, expecting some kind of graceful rejection, or 401. Maybe it needs some error checking? Just guessing. I will test again tomorrow with the token and see what happens.

@flowt-au
Copy link

@RoMiRoSSaN: OK, it seems I had the correct classes and the WebSocketSecurityConfigurator and reactive route is being called when I provide a token. So, progress.

What I cant work out is how to setup the onOpen and onMessage methods so I can access the JWT to get the claims without throwing errors related to blocking. I have tried all sorts of combinations but I can't make sense of how the various bits of documentation fit together.

I have made a SO post about this to surface your helpful code fragments and to seek help on getting it to work.

Thanks again,
Murray

@glmanhtu
Copy link

const ws = new WebSocket(ws://localhost:8080/ws, ["access_token","your token"] );

For other peaople who need auth in WS

Create reactive route

@Singleton
public class WSAuthFilter {
    @RouteFilter(401)
    void addAuthHeader(RoutingContext rc) {
        try {
            if (rc.request().headers().get("Sec-WebSocket-Protocol") != null) {
                String token = "Bearer " + rc.request().headers().get("Sec-WebSocket-Protocol").split(", ")[1];
                rc.request().headers().add("Authorization", token);
            }
        } finally {
            rc.next();
        }
    }
}

Create custom ServerEndpointConfig.Configurator

public class WebSocketSecurityConfigurator extends ServerEndpointConfig.Configurator {
    @Override
    public void modifyHandshake(ServerEndpointConfig sec, HandshakeRequest request, HandshakeResponse response) {
        try {
            String subProtocol = request.getHeaders().get("Sec-WebSocket-Protocol").get(0).split(",")[0];
            response.getHeaders().put("sec-websocket-protocol", List.of(subProtocol));
        } catch (Throwable ignore) {
        }
    }
}

Create WS class

@Authenticated
@ServerEndpoint(
        value ="/ws",
        configurator = WebSocketSecurityConfigurator.class
)
public class WebSocket {
    @Inject
    UserInfo userInfo;
    // .....
}

In JS

const ws = new WebSocket(`ws://localhost:8080/ws`, ["access_token","your token"] );

Confirm that this approach works,

For other people who is still struggling to implement this hack, I created a simple working example here: https://github.com/chat-socket/websocket-server/tree/oauth2-workaround

@flowt-au
Copy link

@glmanhtu Thank you. :-) I will check this out over coming days. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants