Quarkus WebSockets Next does not respect @HttpAuthenticationMechanism

[vmutafov]

Describe the bug

Hi! Please, correct me if I'm wrong, but it seems Quarkus WebSockets Next doesn't handle @HttpAuthenticationMechanism when declared on the web socket class like:

@RolesAllowed("*")
@HttpAuthenticationMechanism(value = "some-custom-mechanism")
@WebSocket(path = "/test")
public class TestWebSocket {
  ...
}

As far as I understand how the @HttpAuthenticationMechanism annotation is handled in regular RestEasy endpoints, usages are recorded here and then the HttpAuthenticator should use the declared auth mechanism.

If there is no pre-selected auth mechanism, all registered auth mechanisms are iterated and requests are checked with every mechanism. In my opinion, this may lead to unexpected security problems if a websocket that should have used a dedicated auth mechanism, now may accept requests as long as they are authenticated by some other auth mechanism.

Expected behavior

The @HttpAuthenticationMechanism should either be supported or at least a build time exception should be thrown when it's declared on a websocket.

Actual behavior

The @HttpAuthenticationMechanism annotation is not handled.

How to Reproduce?

1. Add a custom auth mechanism like in here
2. Create a websocket and make it use the custom auth mechanism
3. Configure some other auth mechanism - I used the Smallrye JWT auth config in my case
4. Try to connect to the websocket with credentials that are not valid for the custom auth mechanism but valid for the other auth mechanism

Output of uname -a or ver

23.1.0 Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64

Output of java -version

openjdk version "21" 2023-09-19 OpenJDK Runtime Environment (build 21+35-2513) OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)

Quarkus version or git rev

3.17.7

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

Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937)

Additional information

No response

[michalvavrik]

That is true, however documentation https://quarkus.io/guides/security-authentication-mechanisms#use-annotations-to-enable-path-based-authentication-for-jakarta-rest-endpoints is clear: Use annotations to enable path-based authentication for Jakarta REST endpoints. WebSockets endpoints are not Jakarta REST endpoints.

@vmutafov if we can settle that this is a feature request, I'll put it on my list and address it eventually :-). I think that "proposal" makes sense as we can detect this before HTTP upgrade checks are performed.

[michalvavrik]

Or if you want to contribute, just shout, I can give you few pointers. At any rate, I think you made a good point.

[michalvavrik]

cc @mkouba @sberyozkin

[michalvavrik]

If there is no pre-selected auth mechanism, all registered auth mechanisms are iterated and requests are checked with every mechanism. In my opinion, this may lead to unexpected security problems if a websocket that should have used a dedicated auth mechanism, now may accept requests as long as they are authenticated by some other auth mechanism.

This is not a first time I was thinking about build-time validation failures for unsupported features in the WS Next, but for permission checkers we decided to implement that instead and I think here it is also better to just start supporting this instead.

[michalvavrik]

Actually, I think I can take care of it rather quickly, I'll have a look tomorrow or next week. Cheers

[vmutafov]

Thanks for your quick responses, @michalvavrik!

That is true, however documentation https://quarkus.io/guides/security-authentication-mechanisms#use-annotations-to-enable-path-based-authentication-for-jakarta-rest-endpoints is clear: Use annotations to enable path-based authentication for Jakarta REST endpoints. WebSockets endpoints are not Jakarta REST endpoints.

@vmutafov if we can settle that this is a feature request, I'll put it on my list and address it eventually :-). I think that "proposal" makes sense as we can detect this before HTTP upgrade checks are performed.

Yeah, it may be considered as a feature request. After reading the docs again, you are right that it's quite clear these annotations are for Jakarta REST but then again, the current behaviour was a bit unexpected (to me at least).

Actually, I think I can take care of it rather quickly, I'll have a look tomorrow or next week. Cheers

I'm also willing to help if I can

[michalvavrik]

Thanks for your quick responses, @michalvavrik!

That is true, however documentation https://quarkus.io/guides/security-authentication-mechanisms#use-annotations-to-enable-path-based-authentication-for-jakarta-rest-endpoints is clear: Use annotations to enable path-based authentication for Jakarta REST endpoints. WebSockets endpoints are not Jakarta REST endpoints.
@vmutafov if we can settle that this is a feature request, I'll put it on my list and address it eventually :-). I think that "proposal" makes sense as we can detect this before HTTP upgrade checks are performed.

Yeah, it may be considered as a feature request. After reading the docs again, you are right that it's quite clear these annotations are for Jakarta REST but then again, the current behaviour was a bit unexpected (to me at least).

got it

Actually, I think I can take care of it rather quickly, I'll have a look tomorrow or next week. Cheers

I'm also willing to help if I can

Yeah, we are really looking for contributors and are quite welcoming https://github.com/quarkusio/quarkus/issues?q=is%3Aissue%20state%3Aopen%20label%3Aarea%2Fsecurity :-) Just I have already opened 3 PRs in WebSockets Next that are waiting for a review and one of them is touching same classes, so I have solution for this, but it doesn't make sense to work on it till this gets unblocked.

[michalvavrik]

I'm also willing to help if I can

I ended-up doing it myself because I remembered it wrong, it was bit complex. #46161 should take care of it. Thanks for reporting this issue, I think it was a good point.