WebSockets Next: fire authorization sucesss and failure events for HTTP upgrade security checks

[michalvavrik]

• when working on Support permission checkers for WebSockets Next #45704 I have mentiond that HTTP upgrade checks don't have security events (docs https://quarkus.io/guides/security-customization#observe-security-events) integrated, hence fixing that
• this PR also adds a smoke tests for the rest of supported authorization scenarios

[sberyozkin]

Thanks Michal, chatbot links can be offered as part of the HTTP application, I wonder, how does the event listener know where exactly it failed, at the HTTP Upgrade time, or at some non-WS HTTP request time ?

[sberyozkin]

The other question, so we recommend using authentication at the HTTP upgrade time, but also allow it on methods like onMessage, after the upgrade, do you plan to have it covered too ?

[michalvavrik]

Thanks Michal, chatbot links can be offered as part of the HTTP application, I wonder, how does the event listener know where exactly it failed, at the HTTP Upgrade time, or at some non-WS HTTP request time ?

This really depends on where is is thrown, sadly I don't have a short answer, so please endure:

It depends on where is authorization failure / success performed:

• The reason why I didn't mention about security events were not fired for HTTP upgrade security checks was that this is the only security thing that WS Next does on it's own. For a good reason, it must be done eagerly and using CDI interceptors is not a good option there.
• the events I am firing in this PR add: endpoint id, HTTP request (so you can get path, headers), security check name; IMHO that is all you need, there is no actual method, so we can't be more specific
• CDI interceptors knows method, if you inspect the test that I added, you will see that authorization events for @PermissionChecker payload tests has class#method because they actually secure some java method; but generally you can't access there routing context or path because it is not available because it is performed in Quarkus Security extension that doesn't know about that (but for the checker we always add RoutingContext so that is exception as well)
• authorization using configuration (HTTP security policies) are adding routingcontext which allows you to inspect path and that again fits configuration, because you configure these checks for some path

Long answer short: you need to inspect event properties. I think we could unify it somehow, but this will always differ based on what is authorized. For example eagerly performed security checks on Quarkus REST endpoints has again different set of properties because there you know: method (it is annotated right?), routingcontext, security check name

The other question, so we recommend using authentication at the HTTP upgrade time, but also allow it on methods like onMessage, after the upgrade, do you plan to have it covered too ?

Not sure what you mean, I think I have tests for everything. if I missed something please let me know and I'll fix it. If you wonder what happens there in regards of security events? every authorization has it's own event, so HTTP Security policies has their own authorization event instance fired and security annotations has their own event instance fired. That's not new, because you can't know if next authorization is coming in a different level, so you can't fire one event per all different authorizations.

[sberyozkin]

@michalvavrik It is just that the PR says HTTP upgrade security checks while in your previous PR, there was a dedicated doc section on security bean methods (after insecure upgrade)

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7521bf2.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[michalvavrik]

@michalvavrik It is just that the PR says HTTP upgrade security checks while in your previous PR, there was a dedicated doc section on security bean methods (after insecure upgrade)

This test method https://github.com/quarkusio/quarkus/pull/45800/files#diff-d45d0fb9a839e6fdf80b00f4e49eea69992399f252c82e5ad3206c231b25ef9dR224 checks security events on bean methods after insecure upgrade.

Sorry if I am slow, it's just that it is something that had worked before, this PR is only fixing events for security annotations placed next to @WebSocket (class level). I am adding tests for other scenarios just to be sure it works. Just let me know if you see something missing and I'll add it.

[michalvavrik]

no rush @mkouba @sberyozkin but this is opened for a review, I don't think there are any discrepancies.

[sberyozkin]

LGTM

[sberyozkin]

@mkouba Hi Martin, would you like to have a look at this PR ?

[mkouba]

Looks good!