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

WebSockets Next: Show how to map request headers to Authorization header that OIDC Bearer token authentication require #45809

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions docs/src/main/asciidoc/websockets-next-reference.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,66 @@ public class ProductEndpoint {
<1> The `getProduct` callback method can only be invoked if the current security identity has an `admin` role or the user is allowed to get the product detail.
<2> The error handler is invoked in case of the authorization failure.

==== Bearer token authentication

The xref:security-oidc-bearer-token-authentication.adoc[OIDC Bearer token authentication] expects that bearer tokens are passed in the `Authorization` header during the initial HTTP handshake.
While we recommend to use WebSocket clients that support setting custom headers in the initial HTTP handshake, there are users that rely on https://websockets.spec.whatwg.org/#the-websocket-interface[WebSockets API].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding that anyone writing JavaScript-based WS applications has no choice but to use that API, right ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it makes sense to have 2 sub-sections, one for WS Client, another for WS API, right now it reads like using WS API is quite a bad idea

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding that anyone writing JavaScript-based WS applications has no choice but to use that API, right ?

My understanding is there are some JS clients that also implements WS and supports that. One that seem mostly trustworthy is websockets/ws#1925 (pointing to the issue because it is clear they are setting headers on the client side) https://github.com/websockets/ws/blob/master/lib/websocket.js#L743. The issue with this is that some users think like I would: do my employer trust this in production and why?

IMHO it makes sense to have 2 sub-sections, one for WS Client, another for WS API, right now it reads like using WS API is quite a bad idea

I thought we need to make very clear line between what is possible and what we suggest (nothing, you are on your own). We can use to use WebSockets API, but I think it requires additional concepts like CORS and custom protocol that has message types and we would authenticate on one of these message types. If you have idea, please suggest specific test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalvavrik IMHO it makes sense to make it clear how users who have no other option but to use WS API can approach it, as opposed to us discouraging them, with some warnings added.
Look at Quarkus LangChain4j demos, chatbots are everywhere, opened from scripts, they can't use Quarkus Java WS Client.
We should not be recommending specific JS client libraries, as under the hood the probably do these workarounds anyway, and I agree, we don't want to take risk and guide users to use specific libraries and then be asked why did we do it...

As far as using WS API is concerned, we should give simple suggestions:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense @sberyozkin , I'll rewrite this later in the afternoon. Thank you

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @michalvavrik, take your time please

In this API, the HTTP `Sec-WebSocket-Protocol` request header is the only header that users can configure.
We have seen users to take advantage of this header for purpose which it is not intended like in the example below:

[source,javascript]
----
const token = getBearerToken()
const socket = new WebSocket("wss://" + location.host + "/chat/" + username, ["bearer", token]); <1>
----
<1> Indicate 2 sub-protocols supported by the client, the `bearer` sub-protocol and the token sub-protocol.

The WebSocket server can also be configured with supported sub-protocols:

[source, properties]
----
quarkus.websockets-next.server.supported-subprotocols=bearer
----
<1> The `bearer` sub-protocol matches the sub-protocol option passed to the client in previous example.

In Quarkus, it is possible to map HTTP server request headers to a different headers, for example:

[source, java]
----
package io.quarkus.websockets.next.test.security;

import static io.quarkus.websockets.next.HandshakeRequest.SEC_WEBSOCKET_PROTOCOL;
import static io.vertx.core.http.HttpHeaders.AUTHORIZATION;

import jakarta.enterprise.event.Observes;

import io.vertx.ext.web.Router;

public class WebSocketTokenHeaderHandler {

private static final String BEARER_PROTOCOL = "bearer";
private static final String TOKEN_PREFIX = BEARER_PROTOCOL + ",";

void observe(@Observes Router router) {
mkouba marked this conversation as resolved.
Show resolved Hide resolved
router.get().order(-250).handler(routingContext -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, would it make sense for WS-Next provide such a handler for users not having to write this code ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #42824 (comment), it can be discussed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO optionally enabling a handler which does a header conversion according the documented scheme is all right, it is not a workaround for users who have to use WS API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think it would be easier to understand than ask users to add this handler. We must be clear in docs about security aspects, that's all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we just explain what users who really need to retain tokens can do, may be we can offer some basic support like having this handler enabled if requests, and ask users take WS API security concerns seriously

String webSocketProtocol = routingContext.request().headers().get(SEC_WEBSOCKET_PROTOCOL);
if (webSocketProtocol != null && webSocketProtocol.startsWith(TOKEN_PREFIX)) {
routingContext.request().headers().remove(SEC_WEBSOCKET_PROTOCOL);
routingContext.request().headers().add(SEC_WEBSOCKET_PROTOCOL, BEARER_PROTOCOL);
String token = webSocketProtocol.substring(TOKEN_PREFIX.length());
routingContext.request().headers().add(AUTHORIZATION, "Bearer " + token);
}
routingContext.next();
});
}

}
----
<1> Set the `Authorization` header to a bearer token provided in a different HTTP request header.

WARNING: Purpose of this example is to show that currently, it is possible to create the `Authorization` header from other request headers.
This way, users that go down this path will know how to configure the server side.

=== Inspect and/or reject HTTP upgrade

To inspect an HTTP upgrade, you must provide a CDI bean implementing the `io.quarkus.websockets.next.HttpUpgradeCheck` interface.
Expand Down
17 changes: 17 additions & 0 deletions integration-tests/oidc-dev-services/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-oidc</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-websockets-next</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5</artifactId>
Expand Down Expand Up @@ -76,6 +80,19 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-websockets-next-deployment</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.quarkus.it.oidc.dev.services;

import jakarta.inject.Inject;

import io.quarkus.security.Authenticated;
import io.quarkus.security.identity.SecurityIdentity;
import io.quarkus.websockets.next.OnOpen;
import io.quarkus.websockets.next.OnTextMessage;
import io.quarkus.websockets.next.WebSocket;

@Authenticated
@WebSocket(path = "/chat/{username}")
public class ChatWebSocket {

@Inject
SecurityIdentity identity;

@OnOpen
public String onOpen() {
return "opened";
}

@OnTextMessage
public String echo(String message) {
return message + " " + identity.getPrincipal().getName();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.quarkus.it.oidc.dev.services;

import static io.quarkus.websockets.next.HandshakeRequest.SEC_WEBSOCKET_PROTOCOL;
import static io.vertx.core.http.HttpHeaders.AUTHORIZATION;

import jakarta.enterprise.event.Observes;

import io.vertx.ext.web.Router;

public class WebSocketTokenHeaderHandler {

private static final String BEARER_PROTOCOL = "bearer";
private static final String TOKEN_PREFIX = BEARER_PROTOCOL + ",";

void observe(@Observes Router router) {
router.get().order(-250).handler(routingContext -> {
String webSocketProtocol = routingContext.request().headers().get(SEC_WEBSOCKET_PROTOCOL);
if (webSocketProtocol != null && webSocketProtocol.startsWith(TOKEN_PREFIX)) {
routingContext.request().headers().remove(SEC_WEBSOCKET_PROTOCOL);
routingContext.request().headers().add(SEC_WEBSOCKET_PROTOCOL, BEARER_PROTOCOL);
String token = webSocketProtocol.substring(TOKEN_PREFIX.length());
routingContext.request().headers().add(AUTHORIZATION, "Bearer " + token);
}
routingContext.next();
});
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
quarkus.oidc.devservices.enabled=true
quarkus.oidc.devservices.roles.Ronald=admin

quarkus.websockets-next.server.supported-subprotocols=bearer

%code-flow.quarkus.oidc.devservices.roles.alice=admin,user
%code-flow.quarkus.oidc.devservices.roles.bob=user
%code-flow.quarkus.oidc.application-type=web-app
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package io.quarkus.it.oidc.dev.services;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.net.URI;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;

import jakarta.inject.Inject;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;

import io.quarkus.test.common.http.TestHTTPResource;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.oidc.client.OidcTestClient;
import io.vertx.core.Vertx;
import io.vertx.core.http.WebSocket;
import io.vertx.core.http.WebSocketClient;
import io.vertx.core.http.WebSocketConnectOptions;

@QuarkusTest
public class WebSocketOidcTest {

@TestHTTPResource("/chat")
URI uri;

@Inject
Vertx vertx;

private static final OidcTestClient oidcTestClient = new OidcTestClient();

@AfterAll
public static void close() {
oidcTestClient.close();
}

@Test
public void testDocumentedTokenPropagationUsingSubProtocol()
throws InterruptedException, ExecutionException, TimeoutException {
// verify that handler documented in WebSockets Next reference
// propagates "Sec-WebSocket-Protocol" as Authorization header
// and authentication is successful
CountDownLatch connectedLatch = new CountDownLatch(1);
CountDownLatch messagesLatch = new CountDownLatch(2);
List<String> messages = new CopyOnWriteArrayList<>();
AtomicReference<WebSocket> ws1 = new AtomicReference<>();
WebSocketClient client = vertx.createWebSocketClient();
WebSocketConnectOptions options = new WebSocketConnectOptions();
options.setHost(uri.getHost());
options.setPort(uri.getPort());
options.setURI(uri.getPath() + "/IF");
options.setSubProtocols(List.of("bearer", oidcTestClient.getAccessToken("alice", "alice")));
try {
client
.connect(options)
.onComplete(r -> {
if (r.succeeded()) {
WebSocket ws = r.result();
ws.textMessageHandler(msg -> {
messages.add(msg);
messagesLatch.countDown();
});
// We will use this socket to write a message later on
ws1.set(ws);
connectedLatch.countDown();
} else {
throw new IllegalStateException(r.cause());
}
});
assertTrue(connectedLatch.await(5, TimeUnit.SECONDS));
ws1.get().writeTextMessage("hello");
assertTrue(messagesLatch.await(5, TimeUnit.SECONDS), "Messages: " + messages);
assertEquals(2, messages.size(), "Messages: " + messages);
assertEquals("opened", messages.get(0));
assertEquals("hello alice", messages.get(1));
} finally {
client.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
}
}

}
Loading