Fix GraphQL WebSocket handling occurring before authorization #36961

rubik-cube-man · 2023-11-08T20:18:01Z

Fixes #20092

It seems that the WebSocket injector for GraphQL occurs before the Authentication and Authorization occur. This leads to GraphQL WebSockets having no user information about the active connection. This change ensures that these steps occur before the WebSocket is constructed so that you can check to see if the active connection has permission to perform operations.

phillip-kruger

LGTM. @jmartisk ?

jmartisk · 2023-11-09T05:49:00Z

Does this really fully resolve #20092? It looks surprisingly simple. Could you demonstrate it by adding a test case? A positive one (where a user can successfully establish a subscription) as well as a negative one (where the request is rejected)

cescoffier · 2023-11-09T06:22:16Z

...eployment/src/main/java/io/quarkus/smallrye/graphql/deployment/SmallRyeGraphQLProcessor.java

@@ -147,7 +143,7 @@ public class SmallRyeGraphQLProcessor {
    private static final List<String> SUPPORTED_WEBSOCKET_SUBPROTOCOLS = List.of(SUBPROTOCOL_GRAPHQL_WS,
            SUBPROTOCOL_GRAPHQL_TRANSPORT_WS);

-    private static final int GRAPHQL_WEBSOCKET_HANDLER_ORDER = -10000;
+    private static final int GRAPHQL_WEBSOCKET_HANDLER_ORDER = ManagementInterfaceFilterBuildItem.AUTHORIZATION + 1;


I don't believe it's the right index. This is for the management interface. We should have a class with the route order constant.

Ahh, yes, it looks like I confused myself with all the positives and negatives. (As filters seem to be inverted from positive to negatives when being added to a route).

Is this the constant that you had in mind? This ensures that WebSocket handling occurs following this step.

...eployment/src/main/java/io/quarkus/smallrye/graphql/deployment/SmallRyeGraphQLProcessor.java

rubik-cube-man · 2023-11-09T13:05:33Z

Does this really fully resolve #20092? It looks surprisingly simple. Could you demonstrate it by adding a test case? A positive one (where a user can successfully establish a subscription) as well as a negative one (where the request is rejected)

Yeah, I agree, after spending two days debugging this, I was pretty surprised that I was able to get it working with such little changes!

I just pushed some unit tests to do security validation of Subscriptions and a few WebSocket queries as well! Are these changes what you had in mind?
I hope there is no problem with me adding the SmallRye GraphQL client as a test dependency. It doesn't seem there are any unit tests for subscriptions as it currently stands.

jmartisk · 2023-11-10T07:52:55Z

I know we're not really "testing" the client here, but since you use the client for this, could you move the test to the client extension? That extension (obviously) depends on the client, so we won't have to pollute the server-side extension with the client dependency (even though it's just for tests, but still I'd say it would be preferable)...
I changed my mind about this, that would require copying the security-related setup too.. Let's keep it in the server module.

I'm planning to have a look today or Monday to give it some manual verification too..
Thanks a lot!

extensions/smallrye-graphql/deployment/pom.xml

jmartisk · 2023-11-10T09:05:25Z

Not sure how you got the tests passing, but when trying manually I still always get

io.vertx.core.http.UpgradeRejectedException: WebSocket upgrade failure: 401

rubik-cube-man · 2023-11-10T10:15:18Z

Not sure how you got the tests passing, but when trying manually I still always get
io.vertx.core.http.UpgradeRejectedException: WebSocket upgrade failure: 401

That's very odd! I did suspect that the connection localhost:8081 was maybe a bad idea. I'll see if I can figure out what might be causing that.
You wouldn't happen to have anything else running on port 8081? I made that mistake yesterday and spent far too long debugging it!

jmartisk · 2023-11-10T12:44:20Z

Ah sorry for the confusion, but.. we really will have to move the tests to the client module. Otherwise we have a cyclic dependency. Unless you get rid of the graphql client and use some kind of pure websocket client, which would be hard, because you would need to re-implement some parts of the subprotocol.

You wouldn't happen to have anything else running on port 8081?

I'm not using the tests, I have a regular application running on 8080 and connecting to it from a different one.
I'll try to look more into it next week, but I am a bit skeptical that this solution is enough.

rubik-cube-man · 2023-11-10T14:32:41Z

Ah sorry for the confusion, but.. we really will have to move the tests to the client module. Otherwise we have a cyclic dependency. Unless you get rid of the graphql client and use some kind of pure websocket client, which would be hard, because you would need to re-implement some parts of the subprotocol.

You wouldn't happen to have anything else running on port 8081?

I'm not using the tests, I have a regular application running on 8080 and connecting to it from a different one. I'll try to look more into it next week, but I am a bit skeptical that this solution is enough.

Ahh, no worries! I'll move those tests at the start of next week!

One thing I forgot to consider until yesterday was that you might want to do multiple clients per WebSocket. Basically, it means that it only works if you set the Authorization in the header of the GraphQL client, I believe. If you are trying to set the header per request, I imagine it won't work.
Unfortunately, I don't have enough experience with Quarkus at the moment to add support for per-request Authentication (unless I copy and paste lots of code).
For us, we only need a single Authentication request per GraphQL client, so this change works for us. For people who may need multiple users authenticated over a single WebSocket, this may not solve their problem, but I guess it provides a workaround for the time being.

rubik-cube-man · 2023-11-12T15:55:35Z

I managed to get to do some further looking into this this weekend. I was successfully able to migrate all the tests over to the client!
I also looked into seeing if I could support adding additional headers on a per-request basis instead of requiring all headers be set before opening the WebSocket. It seems like this isn't even a thing which is supported by SmallRye GraphQL link to code. At the moment I don't think there's anything else I could add, but feel free to let me know if I'm missing anything!

rubik-cube-man · 2023-11-12T23:01:24Z

Just added support for the WebSockets that close themselves when the token associated with the WebSocket expires. Basically same functionality as the Quarkus WebSocket system here. Hopefully this will prevent any WebSocket connections being left open, and all queries against it failing as the token associated with it is no longer valid.

phillip-kruger · 2023-11-12T23:25:06Z

Thanks @rubik-cube-man . Please squash your commits into 1. Thanks

jmartisk · 2023-11-13T07:07:59Z

I also looked into seeing if I could support adding additional headers on a per-request basis instead of requiring all headers be set before opening the WebSocket.

I'm not sure how you imagine this should work - once you open a websocket connection, it's not about requests anymore - it's a bidirectional exchange of messages according to the underlying subprotocol (in our case, graphql-transport-ws). So the identity switching would need to be supported on the subprotocol layer? (It currently doesn't have this, so we would have to extend it somehow).

In any case, supporting one user per connection is a very good start. Thanks a lot for moving this forward.

cescoffier

The mutiny code can be simplified, but that's cosmetic.

...raphql/client/deployment/DynamicGraphQLClientWebSocketAuthenticationHttpPermissionsTest.java

jmartisk · 2023-11-16T08:32:59Z

Hopefully it's all good now, thanks so much @rubik-cube-man for all your effort here. You rock.

jmartisk · 2023-11-16T09:30:02Z

Oh right, the native mode test needs some adjustment, I'm on it

rubik-cube-man · 2023-11-16T11:04:26Z

Hopefully it's all good now, thanks so much @rubik-cube-man for all your effort here. You rock.

No problem! Thanks for all the help as well in getting this ready for release!

quarkus-bot · 2023-11-16T13:57:52Z

Failing Jobs - Building `c1fde71`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✔️	JVM Tests - JDK 17
✖	JVM Tests - JDK 21	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 21 #

- Failing: integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers

📦 integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers

✖ io.quarkus.it.hibernate.multitenancy.fruit.HibernateTenancyFunctionalityTest.testAddDeleteDefaultTenant line 41 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <201> but was <500>.

✖ io.quarkus.it.hibernate.multitenancy.fruit.HibernateTenancyFunctionalityTest.testGetFruitsDefaultTenant line 56 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <200> but was <500>.

✖ io.quarkus.it.hibernate.multitenancy.fruit.HibernateTenancyFunctionalityTest.testGetFruitsTenantMycompany line 66 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <200> but was <500>.

✖ io.quarkus.it.hibernate.multitenancy.fruit.HibernateTenancyFunctionalityTest.testPostFruitDefaultTenant line 80 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <201> but was <500>.

✖ io.quarkus.it.hibernate.multitenancy.fruit.HibernateTenancyFunctionalityTest.testUpdateFruitDefaultTenant line 115 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <201> but was <500>.

gsmet · 2023-11-16T20:13:53Z

Thanks everyone for getting all this in shape!

This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.220.0` -> `^0.222.0`](https://renovatebot.com/diffs/npm/flow-bin/0.220.0/0.222.0) | | [org.postgresql:postgresql](https://jdbc.postgresql.org) ([source](https://github.com/pgjdbc/pgjdbc)) | build | minor | `42.6.0` -> `42.7.0` | | [org.liquibase.ext:liquibase-hibernate5](https://github.com/liquibase/liquibase-hibernate/wiki) ([source](https://github.com/liquibase/liquibase-hibernate)) | build | minor | `4.24.0` -> `4.25.0` | | [org.liquibase:liquibase-maven-plugin](http://www.liquibase.org/liquibase-maven-plugin) ([source](https://github.com/liquibase/liquibase)) | build | minor | `4.24.0` -> `4.25.0` | | [org.jsoup:jsoup](https://jsoup.org/) ([source](https://github.com/jhy/jsoup)) | compile | minor | `1.16.2` -> `1.17.1` | | [io.hypersistence:hypersistence-utils-hibernate-62](https://github.com/vladmihalcea/hypersistence-utils) | compile | patch | `3.6.0` -> `3.6.1` | | [org.hibernate.orm:hibernate-envers](https://hibernate.org/orm) ([source](https://github.com/hibernate/hibernate-orm)) | build | minor | `6.3.1.Final` -> `6.4.0.Final` | | [org.hibernate.orm:hibernate-core](https://hibernate.org/orm) ([source](https://github.com/hibernate/hibernate-orm)) | build | minor | `6.3.1.Final` -> `6.4.0.Final` | | [com.blazebit:blaze-persistence-bom](https://persistence.blazebit.com) ([source](https://github.com/Blazebit/blaze-persistence)) | import | patch | `1.6.9` -> `1.6.10` | | [com.diffplug.spotless:spotless-maven-plugin](https://github.com/diffplug/spotless) | build | minor | `2.40.0` -> `2.41.0` | | [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.5.1` -> `3.6.0` | | [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | patch | `3.5.1` -> `3.5.3` | --- ### Release Notes <details> <summary>flowtype/flow-bin</summary> ### [`v0.222.0`](flow/flow-bin@543cad7...84a68f1) [Compare Source](flow/flow-bin@543cad7...84a68f1) ### [`v0.221.0`](flow/flow-bin@e8b3a2e...543cad7) [Compare Source](flow/flow-bin@e8b3a2e...543cad7) ### [`v0.220.1`](flow/flow-bin@030bfc6...e8b3a2e) [Compare Source](flow/flow-bin@030bfc6...e8b3a2e) </details> <details> <summary>pgjdbc/pgjdbc</summary> ### [`v42.7.0`](https://github.com/pgjdbc/pgjdbc/blob/HEAD/CHANGELOG.md#4270-2023-11-20-093300--0500) ##### Changed - fix: Deprecate for removal PGPoint.setLocation(java.awt.Point) to cut dependency to `java.desktop` module. [MR #2967](pgjdbc/pgjdbc#2967) - feat: return all catalogs for getCatalogs metadata query closes [ISSUE #2949](pgjdbc/pgjdbc#2949) [MR #2953](pgjdbc/pgjdbc#2953) - feat: support SET statements combining with other queries with semicolon in PreparedStatement [MR ##2973](pgjdbc/pgjdbc#2973) ##### Fixed - chore: add styleCheck Gradle task to report style violations [MR #2980](pgjdbc/pgjdbc#2980) - fix: Include currentXid in "Error rolling back prepared transaction" exception message [MR #2978](pgjdbc/pgjdbc#2978) - fix: add varbit as a basic type inside the TypeInfoCache [MR #2960](pgjdbc/pgjdbc#2960) - fix: Fix failing tests for version 16. [MR #2962](pgjdbc/pgjdbc#2962) - fix: allow setting arrays with ANSI type name [MR #2952](pgjdbc/pgjdbc#2952) - feat: Use KeepAlive to confirm LSNs [MR #2941](pgjdbc/pgjdbc#2941) - fix: put double ' around log parameter [MR #2936](pgjdbc/pgjdbc#2936) fixes [ISSUE #2935](pgjdbc/pgjdbc#2935) - fix: Fix Issue [#2928](pgjdbc/pgjdbc#2928) number of ports not equal to number of servers in datasource [MR #2929](pgjdbc/pgjdbc#2929) - fix: Use canonical DateStyle name ([#2925](pgjdbc/pgjdbc#2925)) fixes [pgbouncer issue](pgbouncer/pgbouncer#776) - fix: Method getFastLong should be able to parse all longs [MR #2881](pgjdbc/pgjdbc#2881) - docs: Fix typos in info.html [MR #2860](pgjdbc/pgjdbc#2860) - fix: Return correct default from PgDatabaseMetaData.getDefaultTransactionIsolation [MR #2992](pgjdbc/pgjdbc#2992) fixes [Issue #2991](pgjdbc/pgjdbc#2991) - test: fix assertion in RefCursorFetchTestultFetchSize rows - test: use try-with-resources in LogicalReplicationStatusTest </details> <details> <summary>liquibase/liquibase-hibernate</summary> ### [`v4.25.0`](https://github.com/liquibase/liquibase-hibernate/releases/tag/v4.25.0): Support for Liquibase Hibernate 6 Extension v4.25.0 [Compare Source](liquibase/liquibase-hibernate@v4.24.0...v4.25.0) #### Changes #### What's Changed - DAT-15993 - liquibase-hibernate using Liquibase Parent POM by [@jandroav](https://github.com/jandroav) in liquibase/liquibase-hibernate#587 - Update README.md by [@vivekBoii](https://github.com/vivekBoii) in liquibase/liquibase-hibernate#585 - Update pom.xml by [@jandroav](https://github.com/jandroav) in liquibase/liquibase-hibernate#596 - chore(deps): bump liquibase/build-logic from 0.4.7 to 0.5.5 by [@dependabot](https://github.com/dependabot) in liquibase/liquibase-hibernate#609 - Fixed a typo in ReadMe by [@smty2018](https://github.com/smty2018) in liquibase/liquibase-hibernate#600 #### New Contributors - [@vivekBoii](https://github.com/vivekBoii) made their first contribution in liquibase/liquibase-hibernate#585 - [@smty2018](https://github.com/smty2018) made their first contribution in liquibase/liquibase-hibernate#600 - [@sayaliM0412](https://github.com/sayaliM0412) made their first contribution in liquibase/liquibase-hibernate#618 **Full Changelog**: liquibase/liquibase-hibernate@v4.24.0...v4.25.0 </details> <details> <summary>liquibase/liquibase</summary> ### [`v4.25.0`](https://github.com/liquibase/liquibase/blob/HEAD/changelog.txt#Liquibase-4250-is-a-major-release) [Compare Source](liquibase/liquibase@v4.24.0...v4.25.0) </details> <details> <summary>vladmihalcea/hypersistence-utils</summary> ### [`v3.6.1`](https://github.com/vladmihalcea/hypersistence-utils/blob/HEAD/changelog.txt#Version-361---November-11-2023) \================================================================================ Export the testing mechanism [#676](vladmihalcea/hypersistence-utils#676) </details> <details> <summary>hibernate/hibernate-orm</summary> ### [`v6.4.0.Final`](https://github.com/hibernate/hibernate-orm/blob/HEAD/changelog.txt#Changes-in-640Final-November-23-2023) [Compare Source](hibernate/hibernate-orm@6.3.2...6.4.0) https://hibernate.atlassian.net/projects/HHH/versions/32212 \*\* Bug \* \[HHH-17454] - SemanticException caused by type check when comparing generic path to parameter expression \* \[HHH-17428] - Parameter place holder should start from 1 in StandardTemporaryTableExporter \* \[HHH-17415] - NullPointerException: EntityValuedPathInterpretation - getNavigablePath() \* \[HHH-17412] - Type comparison error due to surprising javac method selection \* \[HHH-17411] - Fetch join on treated join leads to owner not selected error \* \[HHH-17386] - Type inference source is not reset for top level predicates \* \[HHH-17384] - OneToMany association with [@NotFound](https://github.com/NotFound) results in SQL with different JOIN-type for SELECT (LEFT JOIN) and COUNT (JOIN) \* \[HHH-17383] - Association is null in lazy initialized element collection \* \[HHH-17382] - Dynamic instantiation leads to superclass fields not found when using injection \* \[HHH-17381] - fix wrong groupId in Compatibility.adoc \* \[HHH-17380] - Persisting an entity with a non generated id and [@MapsId](https://github.com/MapsId) throws PropertyValueException \* \[HHH-17370] - ServiceException: Unable to create requested service \[org.hibernate.engine.jdbc.env.spi.JdbcEnvironment] due to: Cannot invoke "org.hibernate.resource.jdbc.spi.JdbcObserver.jdbcConnectionAcquisitionEnd(java.sql.Connection)" because "this.observer" is null \* \[HHH-17344] - DB2zDialect NullPointerException \* \[HHH-17328] - Retrieve entity using entity graph not adding type in the where clause for [@Inheritance](https://github.com/Inheritance)(strategy = InheritanceType.SINGLE_TABLE) \* \[HHH-17313] - Session#setDefaultReadOnly is ignored by named queries \* \[HHH-17308] - AssertionError when mixing [@SQLSelect](https://github.com/SQLSelect) and composite ID \* \[HHH-17299] - AssertionError in DiscriminatorPathInterpretation when treating a path with the same subtype \* \[HHH-17294] - Non-Embeddable JSON objects are not marked as dirty when modified \* \[HHH-17292] - MappedSuperclass with more than 1 subclass level leads to "UnknownPathException: Could not resolve attribute" \* \[HHH-17102] - [@SqlResultSetMapping](https://github.com/SqlResultSetMapping) doesn’t work with [@Inheritance](https://github.com/Inheritance)(strategy = InheritanceType.JOINED) \*\* Deprecation \* \[HHH-17441] - Deprecate [@Comment](https://github.com/Comment) \*\* Improvement \* \[HHH-17425] - Introduce new configuration parameters for offline Dialect initialization \* \[HHH-17424] - Have Dialect manage more of ExtractedDatabaseMetadata \* \[HHH-17417] - Workaround Oracle driver issue to reduce connection creation \* \[HHH-17409] - Support offset without limit in AbstractSimpleLimitHandler and Oracle12LimitHandler \* \[HHH-17389] - Add getQueryHintString() for PostgreSQLDialect \* \[HHH-17372] - Endless recursion between default implementations of SelectionQuery.getResultStream() and SelectionQuery.stream() \* \[HHH-17355] - Smoothen rough edges with array functions \* \[HHH-17340] - Fix typos in javadoc \* \[HHH-17023] - Add support for Altibase dialect \* \[HHH-15074] - Allow partial composite id generation for EmbeddedId \*\* New Feature \* \[HHH-17357] - Support pgvector types and functions \* \[HHH-17210] - Expose custom JFR events \*\* Sub-task \* \[HHH-17347] - Support for JDK which do not support JFR events \*\* Task \* \[HHH-17390] - Change scope of AbstyractEntityInitializer#resolveInstance \* \[HHH-17367] - Add links to tutorials in documentation \* \[HHH-17362] - Define dependencies of hibernate-jpamodelgen as api \* \[HHH-17350] - Work on hibernate-models, XSD and JAXB ### [`v6.3.2.Final`](hibernate/hibernate-orm@6.3.1...6.3.2) [Compare Source](hibernate/hibernate-orm@6.3.1...6.3.2) </details> <details> <summary>Blazebit/blaze-persistence</summary> ### [`v1.6.10`](https://github.com/Blazebit/blaze-persistence/blob/HEAD/CHANGELOG.md#1610) [Compare Source](Blazebit/blaze-persistence@1.6.9...1.6.10) 12/11/2023 - [Release tag](https://github.com/Blazebit/blaze-persistence/releases/tag/1.6.10) [Resolved issues](https://github.com/Blazebit/blaze-persistence/issues?q=is%3Aissue+milestone%3A1.6.10+is%3Aclosed+sort%3Aupdated-desc) ##### New features - Support JDK 21 - Add CockroachDB function registrations - Support Hibernate 6.3 and 6.4 - Special case limit 1 in correlation builders to support old MySQL and MariaDB versions ##### Bug fixes - Fix parsing error for entity view limit mapping - Fix dropping of embeddable group by expression when nested property has same name as parent property - Fix SQL generation for lateral subqueries when correlated path has `@Where` predicate - Fix `ConcurrentModificationException` during metamodel determination for special Hibernate Envers mappings - Clear `EntityViewManager` static fields in entity view implementations to avoid possible memory leak - Ignore `@Any` mapped attributes in enum type scanning - Fix NPE caused by wrong order by expression during criteria builder copying - Workaround Hibernate 6 returning null java type for enum parameters - Add Entity View type test values for more Java types ##### Backwards-incompatible changes None yet </details> <details> <summary>diffplug/spotless</summary> ### [`v2.41.0`](https://github.com/diffplug/spotless/blob/HEAD/CHANGES.md#2410---2023-08-29) ##### Added - Add a `jsonPatch` step to `json` formatter configurations. This allows patching of JSON documents using [JSON Patches](https://jsonpatch.com). ([#1753](diffplug/spotless#1753)) - Support GJF own import order. ([#1780](diffplug/spotless#1780)) ##### Fixed - Use latest versions of popular style guides for `eslint` tests to fix failing `useEslintXoStandardRules` test. ([#1761](diffplug/spotless#1761), [#1756](diffplug/spotless#1756)) - Add support for `prettier` version `3.0.0` and newer. ([#1760](diffplug/spotless#1760), [#1751](diffplug/spotless#1751)) - Fix npm install calls when npm cache is not up-to-date. ([#1760](diffplug/spotless#1760), [#1750](diffplug/spotless#1750)) ##### Changes - Bump default `eslint` version to latest `8.31.0` -> `8.45.0` ([#1761](diffplug/spotless#1761)) - Bump default `prettier` version to latest (v2) `2.8.1` -> `2.8.8`. ([#1760](diffplug/spotless#1760)) - Bump default `greclipse` version to latest `4.27` -> `4.28`. ([#1775](diffplug/spotless#1775)) </details> <details> <summary>quarkusio/quarkus</summary> ### [`v3.6.0`](quarkusio/quarkus@3.5.3...3.6.0) [Compare Source](quarkusio/quarkus@3.5.3...3.6.0) ### [`v3.5.3`](https://github.com/quarkusio/quarkus/releases/tag/3.5.3) [Compare Source](quarkusio/quarkus@3.5.2...3.5.3) ##### Complete changelog - [#37215](quarkusio/quarkus#37215) - Use LinkedHashMap for parts map to ensure user input order - [#37214](quarkusio/quarkus#37214) - MultipartFormDataOutput should use an ordered map instead of a HashMap - [#37210](quarkusio/quarkus#37210) - \[3.5] Fix and adjust Quarkiverse extension template - [#37209](quarkusio/quarkus#37209) - Build cache - Additional tweaks - [#37206](quarkusio/quarkus#37206) - recognize quarkus.tls.trust-all property by keycloak-admin-client extension - [#37174](quarkusio/quarkus#37174) - Ignore files coming from quarkus-ide-launcher jar - [#37130](quarkusio/quarkus#37130) - Do not report unused deprecated runtime props with default value as used - [#37102](quarkusio/quarkus#37102) - Fix filter per extension in dev ui - [#37073](quarkusio/quarkus#37073) - Use 3.2 as the example stream for update-quarkus.adoc - [#37072](quarkusio/quarkus#37072) - Deprecated runtime configuration properties with default value are reported even though never used - [#37046](quarkusio/quarkus#37046) - Adjust Quarkiverse Antora doc templates a bit - [#36961](quarkusio/quarkus#36961) - Fix GraphQL WebSocket handling occurring before authorization ### [`v3.5.2`](https://github.com/quarkusio/quarkus/releases/tag/3.5.2) [Compare Source](quarkusio/quarkus@3.5.1...3.5.2) ##### Complete changelog - [#37120](quarkusio/quarkus#37120) - Bump Smallrye RM from 4.10.1 to 4.10.2 - [#37104](quarkusio/quarkus#37104) - Make analytics tests more a bit more resilient - [#37090](quarkusio/quarkus#37090) - Add the actual coordinates of the MySQL driver - [#37070](quarkusio/quarkus#37070) - Security doc fix: Broken link and bad code snippet - [#37069](quarkusio/quarkus#37069) - Tiny tweaks based on QE feedback for Datasource guide - [#37068](quarkusio/quarkus#37068) - Updates infinispan client intelligence section - [#37058](quarkusio/quarkus#37058) - Bump com.fasterxml.jackson:jackson-bom from 2.15.2 to 2.15.3 - [#37055](quarkusio/quarkus#37055) - Bump io.smallrye.config:smallrye-config-source-yaml from 3.4.1 to 3.4.4 in /devtools/gradle - [#37038](quarkusio/quarkus#37038) - Disable CustomManifestArgumentsTest on Windows - [#37032](quarkusio/quarkus#37032) - OpenAPI make sure basic auth auto detection work - [#37028](quarkusio/quarkus#37028) - Fix typos in reactive-sql-clients.adoc - [#37025](quarkusio/quarkus#37025) - Document how to log authentication failures for RESTEasy Reactive users migrating from the RESTEasy Classic - [#37019](quarkusio/quarkus#37019) - Address CVE-2023-21971 present in MySQL connector - [#37018](quarkusio/quarkus#37018) - Address CVE-2023-21971 present in MySQL connector - [#37015](quarkusio/quarkus#37015) - Bump org.eclipse.parsson:parsson from 1.1.4 to 1.1.5 - [#37010](quarkusio/quarkus#37010) - Fix vale errors and some warnings in the OIDC Configuration Properties reference guide - [#37006](quarkusio/quarkus#37006) - Never register server specific providers in REST Client (fixed) - [#37003](quarkusio/quarkus#37003) - Small adjustments for documentation related content - [#37001](quarkusio/quarkus#37001) - Revert "Unblock SmallRye Health exposed routes" - [#36991](quarkusio/quarkus#36991) - Upgrade es-module-shims to 1.8.1 - [#36985](quarkusio/quarkus#36985) - Generate a file with relations between guides - [#36983](quarkusio/quarkus#36983) - Fix discarded ObjectMapper configuration - [#36981](quarkusio/quarkus#36981) - Updates to Infinispan 14.0.20.Final - [#36968](quarkusio/quarkus#36968) - Send host.name in all spans - [#36953](quarkusio/quarkus#36953) - Workaround quarkusio/quarkus#36952 alias jboss/jboss-parent-pom#236 jboss-parent:40 still manages jdk-misc, but does not define version.jdk-misc anymore - [#36942](quarkusio/quarkus#36942) - Option TraceServiceLoaderFeature removed in GraalVM 23.1 - [#36941](quarkusio/quarkus#36941) - Fix OTel Resource Attributes - [#36924](quarkusio/quarkus#36924) - Add keywords and topics for hibernate-search-orm-elasticsearch.adoc - [#36917](quarkusio/quarkus#36917) - Update SmallRye Config to 3.4.4 - [#36914](quarkusio/quarkus#36914) - Reset databases/users for each Hibernate ORM tenancy test module - [#36912](quarkusio/quarkus#36912) - Avoid `@TempDir` in RestClientCDIDelegateBuilderTest - [#36884](quarkusio/quarkus#36884) - SmallRye Config property mapping mismatches from the property name in the generated config documentation - [#36868](quarkusio/quarkus#36868) - Native binary generated using quarkus, graalvm and picocli trying to read from .env folder in working directory - [#36856](quarkusio/quarkus#36856) - Lowercase env vars with hyphens are no picked up anymore in Quarkus 3.5.0 (e.g. in docker compose or Hashicorp Nomad) - [#36850](quarkusio/quarkus#36850) - ObjectMapper configuration is discarded in resteasy-reactive-jackson's JsonFactory - [#36847](quarkusio/quarkus#36847) - SmallRye Config error message suggests strange enum values when a bad value is passed - [#36753](quarkusio/quarkus#36753) - Fix order of defaults recording - [#36742](quarkusio/quarkus#36742) - DevUI resource error on main - [#36711](quarkusio/quarkus#36711) - Disable Http2RSTFloodProtectionTest on Windows - [#36578](quarkusio/quarkus#36578) - Maven CLI: add startWith matching into recipes detection - [#36573](quarkusio/quarkus#36573) - Maven CLI: add wildcard matching into recipes detection - [#36570](quarkusio/quarkus#36570) - Maven CLI: use recipes for platform extensions - [#36568](quarkusio/quarkus#36568) - Maven CLI: use recipes for platform extensions - [#36129](quarkusio/quarkus#36129) - auto-service-loader-registration fails with GraalVM CE 21+35.1 - [#32049](quarkusio/quarkus#32049) - resteasy-reactive Interceptors don't get executed after Authorization failure - [#31024](quarkusio/quarkus#31024) - Resteasy Reactive client tries to use ContainerResponseFilter </details> <details> <summary>quarkusio/quarkus-platform</summary> ### [`v3.5.3`](quarkusio/quarkus-platform@3.5.2...3.5.3) [Compare Source](quarkusio/quarkus-platform@3.5.2...3.5.3) ### [`v3.5.2`](quarkusio/quarkus-platform@3.5.1...3.5.2) [Compare Source](quarkusio/quarkus-platform@3.5.1...3.5.2) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

jkylekelly · 2023-12-23T00:51:35Z

Hey all, I noticed in the advisory that the affected package is io.quarkus:quarkus-smallrye-graphql-client-parent. But wouldn't the affected package actually be io.quarkus:quarkus-smallrye-graphql since it'd be the server offering the vulnerable GraphQL operation?

Or is it truly the client-side authorization that is the vulnerability here?

Appreciate any help with this! Admittedly, I'm not very familiar with the project.

jmartisk · 2024-01-02T07:51:32Z

Hey all, I noticed in the advisory that the affected package is io.quarkus:quarkus-smallrye-graphql-client-parent. But wouldn't the affected package actually be io.quarkus:quarkus-smallrye-graphql since it'd be the server offering the vulnerable GraphQL operation?

Or is it truly the client-side authorization that is the vulnerability here?

Appreciate any help with this! Admittedly, I'm not very familiar with the project.

You're correct, this is a server-side issue. I'm not sure where GitHub took the wrong metadata from.

rsvoboda · 2024-01-02T09:15:42Z

@maxandersen @cescoffier (as you have some experience with GHSA) could you look into GHSA-mvc8-6ffp-jrx5 why it mentions quarkus-smallrye-graphql-client-parent ?

cescoffier · 2024-01-02T09:20:58Z

@rsvoboda I've submitted an update.

tasso94 · 2024-02-01T16:00:08Z

Why is this not backported to the 3.2 LTS version?

jmartisk · 2024-02-01T17:40:29Z

@tasso94 it is, it's fixed in 3.2.9 (by this commit: 3550659)

polarctos · 2024-02-02T15:01:01Z

@tasso94 it is, it's fixed in 3.2.9 (by this commit: 3550659)

How can this fixed version information for 3.2.9 be added to the various databases? Currently dependency scanners also flag the Quarkus LTS 3.2.10 as vulnerable, based on incomplete patch version data.

GitHub as of today says patched is >= 3.5.3, 2.13.9.Final: GHSA-mvc8-6ffp-jrx5
NVD data says only fixed in 3.6 and targets whole "quarkus:quarkus": https://nvd.nist.gov/vuln/detail/CVE-2023-6394

RedHat page shows 3.2.9 as patched correctly: https://access.redhat.com/security/cve/cve-2023-6394

tasso94 · 2024-02-05T08:21:32Z

Another question: why is this fixed in Quarkus 2.13 when 2.16 is the latest 2.X release? Is 2.13 an LTS version as well?

jmartisk · 2024-02-05T08:58:59Z

I think @cescoffier can update the github CVE database with 3.2.9 as patched?!

Another question: why is this fixed in Quarkus 2.13 when 2.16 is the latest 2.X release? Is 2.13 an LTS version as well?

Yeah 2.13 is LTS and released as a Red Hat-supported product. @gsmet do we want to fix it in 2.16 too? I'm not sure we are planning any more 2.16 releases, but this actually seems important enough

quarkus-bot bot added area/graphql area/smallrye labels Nov 8, 2023

sberyozkin requested review from jmartisk and phillip-kruger November 8, 2023 23:25

phillip-kruger approved these changes Nov 9, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

cescoffier requested changes Nov 9, 2023

View reviewed changes

quarkus-bot bot added area/flyway area/redis labels Nov 9, 2023

rubik-cube-man force-pushed the smallrye-graphql-websocket-authorization branch from 760ba55 to 619495d Compare November 9, 2023 10:08

This comment has been minimized.

Sign in to view

jmartisk reviewed Nov 10, 2023

View reviewed changes

extensions/smallrye-graphql/deployment/pom.xml Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rubik-cube-man force-pushed the smallrye-graphql-websocket-authorization branch from 77788eb to fd2780c Compare November 12, 2023 15:45

rubik-cube-man force-pushed the smallrye-graphql-websocket-authorization branch 2 times, most recently from 071e9b2 to a4629d7 Compare November 12, 2023 23:12

This comment has been minimized.

Sign in to view

rubik-cube-man force-pushed the smallrye-graphql-websocket-authorization branch 2 times, most recently from dcf2691 to 089ba19 Compare November 13, 2023 10:12

cescoffier and others added 2 commits November 16, 2023 08:15

Use Vert.x instace of Netty to schedule the closing task

a5246bf

Add test for graphql over websockets with only HTTP-based permissions

631a307

cescoffier approved these changes Nov 16, 2023

View reviewed changes

...raphql/client/deployment/DynamicGraphQLClientWebSocketAuthenticationHttpPermissionsTest.java Show resolved Hide resolved

jmartisk approved these changes Nov 16, 2023

View reviewed changes

Fix native mode GraphQLAuthExpiryIT

c1fde71

gsmet merged commit 25ee3be into quarkusio:main Nov 16, 2023
50 of 51 checks passed

quarkus-bot bot added this to the 3.7 - main milestone Nov 16, 2023

quarkus-bot bot added the kind/bugfix label Nov 16, 2023

gsmet added triage/backport? labels Nov 16, 2023

gsmet modified the milestones: 3.7 - main, 3.5.3 Nov 16, 2023

gsmet removed triage/backport-3.5? labels Nov 16, 2023

soumyadip007 mentioned this pull request Jan 9, 2024

fix(sidecar) : quarkus platform version update & graphql vulnerability fixed spaship/sidecar#17

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix GraphQL WebSocket handling occurring before authorization #36961

Fix GraphQL WebSocket handling occurring before authorization #36961

rubik-cube-man commented Nov 8, 2023

phillip-kruger left a comment

This comment has been minimized.

jmartisk commented Nov 9, 2023 •

edited

Loading

cescoffier Nov 9, 2023

rubik-cube-man Nov 9, 2023

rubik-cube-man commented Nov 9, 2023

This comment has been minimized.

jmartisk commented Nov 10, 2023 •

edited

Loading

jmartisk commented Nov 10, 2023

rubik-cube-man commented Nov 10, 2023

This comment has been minimized.

jmartisk commented Nov 10, 2023 •

edited

Loading

rubik-cube-man commented Nov 10, 2023

rubik-cube-man commented Nov 12, 2023

rubik-cube-man commented Nov 12, 2023

phillip-kruger commented Nov 12, 2023

This comment has been minimized.

jmartisk commented Nov 13, 2023

cescoffier left a comment

jmartisk commented Nov 16, 2023

jmartisk commented Nov 16, 2023

rubik-cube-man commented Nov 16, 2023

quarkus-bot bot commented Nov 16, 2023

gsmet commented Nov 16, 2023

jkylekelly commented Dec 23, 2023

jmartisk commented Jan 2, 2024

rsvoboda commented Jan 2, 2024

cescoffier commented Jan 2, 2024

tasso94 commented Feb 1, 2024

jmartisk commented Feb 1, 2024

polarctos commented Feb 2, 2024

tasso94 commented Feb 5, 2024

jmartisk commented Feb 5, 2024

Fix GraphQL WebSocket handling occurring before authorization #36961

Fix GraphQL WebSocket handling occurring before authorization #36961

Conversation

rubik-cube-man commented Nov 8, 2023

phillip-kruger left a comment

Choose a reason for hiding this comment

This comment has been minimized.

jmartisk commented Nov 9, 2023 • edited Loading

cescoffier Nov 9, 2023

Choose a reason for hiding this comment

rubik-cube-man Nov 9, 2023

Choose a reason for hiding this comment

rubik-cube-man commented Nov 9, 2023

This comment has been minimized.

jmartisk commented Nov 10, 2023 • edited Loading

jmartisk commented Nov 10, 2023

rubik-cube-man commented Nov 10, 2023

This comment has been minimized.

jmartisk commented Nov 10, 2023 • edited Loading

rubik-cube-man commented Nov 10, 2023

rubik-cube-man commented Nov 12, 2023

rubik-cube-man commented Nov 12, 2023

phillip-kruger commented Nov 12, 2023

This comment has been minimized.

jmartisk commented Nov 13, 2023

cescoffier left a comment

Choose a reason for hiding this comment

jmartisk commented Nov 16, 2023

jmartisk commented Nov 16, 2023

rubik-cube-man commented Nov 16, 2023

quarkus-bot bot commented Nov 16, 2023

Failing Jobs - Building c1fde71

Failures

⚙️ JVM Tests - JDK 21 #

📦 integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers

gsmet commented Nov 16, 2023

jkylekelly commented Dec 23, 2023

jmartisk commented Jan 2, 2024

rsvoboda commented Jan 2, 2024

cescoffier commented Jan 2, 2024

tasso94 commented Feb 1, 2024

jmartisk commented Feb 1, 2024

polarctos commented Feb 2, 2024

tasso94 commented Feb 5, 2024

jmartisk commented Feb 5, 2024

jmartisk commented Nov 9, 2023 •

edited

Loading

jmartisk commented Nov 10, 2023 •

edited

Loading

jmartisk commented Nov 10, 2023 •

edited

Loading

Failing Jobs - Building `c1fde71`