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

Fix GraphQL WebSocket handling occurring before authorization #36961

Merged
Merged
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
2 changes: 1 addition & 1 deletion .github/native-tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
{
"category": "Misc3",
"timeout": 80,
"test-modules": "kubernetes-client, openshift-client, kubernetes-service-binding-jdbc, smallrye-config, smallrye-graphql, smallrye-graphql-client, smallrye-metrics",
"test-modules": "kubernetes-client, openshift-client, kubernetes-service-binding-jdbc, smallrye-config, smallrye-graphql, smallrye-graphql-client, smallrye-graphql-client-keycloak, smallrye-metrics",
"os-name": "ubuntu-latest"
},
{
Expand Down
10 changes: 10 additions & 0 deletions extensions/smallrye-graphql-client/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@
<artifactId>stork-service-discovery-static-list</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-elytron-security-deployment</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-elytron-security-properties-file-deployment</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package io.quarkus.smallrye.graphql.client.deployment;

import jakarta.annotation.security.RolesAllowed;

import org.eclipse.microprofile.graphql.GraphQLApi;
import org.eclipse.microprofile.graphql.Query;
import org.jboss.shrinkwrap.api.asset.EmptyAsset;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.common.annotation.NonBlocking;
import io.smallrye.graphql.api.Subscription;
import io.smallrye.graphql.client.Response;
import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient;
import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClientBuilder;
import io.smallrye.mutiny.Multi;
import io.smallrye.mutiny.helpers.test.AssertSubscriber;
import io.vertx.core.http.UpgradeRejectedException;

/**
* Due to the complexity of establishing a WebSocket, WebSocket/Subscription testing of the GraphQL server is done here,
* as the client framework comes in very useful for establishing the connection to the server.
* <br>
* This test establishes connections to the server, and ensures that the connected user has the necessary permissions to
* execute the operation.
*/
public class DynamicGraphQLClientWebSocketAuthenticationHttpPermissionsTest {

static String url = "http://" + System.getProperty("quarkus.http.host", "localhost") + ":" +
System.getProperty("quarkus.http.test-port", "8081") + "/graphql";

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(SecuredApi.class, Foo.class)
.addAsResource("application-secured-http-permissions.properties", "application.properties")
.addAsResource("users.properties")
.addAsResource("roles.properties")
.addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml"));

@Disabled("TODO: enable after upgrade to smallrye-graphql 1.6.1, with 1.6.0 a websocket upgrade failure causes a hang here")
@Test
public void testUnauthenticatedForQueryWebSocket() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.executeSingleOperationsOverWebsocket(true);
try (DynamicGraphQLClient client = clientBuilder.build()) {
try {
client.executeSync("{ baz { message} }");
Assertions.fail("WebSocket upgrade should fail");
} catch (UpgradeRejectedException e) {
// ok
}
}
}

@Test
public void testUnauthenticatedForSubscriptionWebSocket() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url);
try (DynamicGraphQLClient client = clientBuilder.build()) {
AssertSubscriber<Response> subscriber = new AssertSubscriber<>();
client.subscription("{ bazSub { message} }").subscribe().withSubscriber(subscriber);
subscriber.awaitFailure().assertFailedWith(UpgradeRejectedException.class);
}
}

public static class Foo {

private String message;

public Foo(String foo) {
this.message = foo;
}

public String getMessage() {
return message;
}

public void setMessage(String message) {
this.message = message;
}

}

@GraphQLApi
public static class SecuredApi {

@Query
@RolesAllowed("fooRole")
@NonBlocking
public Foo foo() {
return new Foo("foo");
}

@Query
@RolesAllowed("barRole")
public Foo bar() {
return new Foo("bar");
}

@Query
public Foo baz() {
return new Foo("baz");
}

@Subscription
@RolesAllowed("fooRole")
public Multi<Foo> fooSub() {
return Multi.createFrom().emitter(emitter -> {
emitter.emit(new Foo("foo"));
emitter.complete();
});
}

@Subscription
@RolesAllowed("barRole")
public Multi<Foo> barSub() {
return Multi.createFrom().emitter(emitter -> {
emitter.emit(new Foo("bar"));
emitter.complete();
});
}

@Subscription
public Multi<Foo> bazSub() {
return Multi.createFrom().emitter(emitter -> {
jmartisk marked this conversation as resolved.
Show resolved Hide resolved
emitter.emit(new Foo("baz"));
emitter.complete();
});
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
package io.quarkus.smallrye.graphql.client.deployment;

import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.concurrent.atomic.AtomicBoolean;

import jakarta.annotation.security.RolesAllowed;
import jakarta.json.JsonValue;

import org.eclipse.microprofile.graphql.GraphQLApi;
import org.eclipse.microprofile.graphql.Query;
import org.jboss.shrinkwrap.api.asset.EmptyAsset;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.common.annotation.NonBlocking;
import io.smallrye.graphql.api.Subscription;
import io.smallrye.graphql.client.Response;
import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient;
import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClientBuilder;
import io.smallrye.mutiny.Multi;

/**
* Due to the complexity of establishing a WebSocket, WebSocket/Subscription testing of the GraphQL server is done here,
* as the client framework comes in very useful for establishing the connection to the server.
* <br>
* This test establishes connections to the server, and ensures that the connected user has the necessary permissions to
* execute the operation.
*/
public class DynamicGraphQLClientWebSocketAuthenticationTest {

static String url = "http://" + System.getProperty("quarkus.http.host", "localhost") + ":" +
System.getProperty("quarkus.http.test-port", "8081") + "/graphql";

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(SecuredApi.class, Foo.class)
.addAsResource("application-secured.properties", "application.properties")
.addAsResource("users.properties")
.addAsResource("roles.properties")
.addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml"));

@Test
public void testAuthenticatedUserForSubscription() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.header("Authorization", "Basic ZGF2aWQ6cXdlcnR5MTIz");
try (DynamicGraphQLClient client = clientBuilder.build()) {
Multi<Response> subscription = client
.subscription("subscription fooSub { fooSub { message } }");

assertNotNull(subscription);

AtomicBoolean hasData = new AtomicBoolean(false);
AtomicBoolean hasCompleted = new AtomicBoolean(false);

subscription.subscribe().with(item -> {
assertFalse(hasData.get());
assertTrue(item.hasData());
assertEquals(JsonValue.ValueType.OBJECT, item.getData().get("fooSub").getValueType());
assertEquals("foo", item.getData().getJsonObject("fooSub").getString("message"));
hasData.set(true);
}, Assertions::fail, () -> {
hasCompleted.set(true);
});

await().untilTrue(hasCompleted);
assertTrue(hasData.get());
}
}

@Test
public void testAuthenticatedUserForQueryWebSocket() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.header("Authorization", "Basic ZGF2aWQ6cXdlcnR5MTIz")
.executeSingleOperationsOverWebsocket(true);
try (DynamicGraphQLClient client = clientBuilder.build()) {
Response response = client.executeSync("{ foo { message} }");
assertTrue(response.hasData());
assertEquals("foo", response.getData().getJsonObject("foo").getString("message"));
}
}

@Test
public void testAuthorizedAndUnauthorizedForQueryWebSocket() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.header("Authorization", "Basic ZGF2aWQ6cXdlcnR5MTIz")
.executeSingleOperationsOverWebsocket(true);
try (DynamicGraphQLClient client = clientBuilder.build()) {
Response response = client.executeSync("{ foo { message} }");
assertTrue(response.hasData());
assertEquals("foo", response.getData().getJsonObject("foo").getString("message"));

// Run a second query with a different result to validate that the result of the first query isn't being cached at all.
response = client.executeSync("{ bar { message} }");
assertEquals(JsonValue.ValueType.NULL, response.getData().get("bar").getValueType());
}
}

@Test
public void testUnauthorizedUserForSubscription() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.header("Authorization", "Basic ZGF2aWQ6cXdlcnR5MTIz");
try (DynamicGraphQLClient client = clientBuilder.build()) {
Multi<Response> subscription = client
.subscription("subscription barSub { barSub { message } }");

assertNotNull(subscription);

AtomicBoolean returned = new AtomicBoolean(false);

subscription.subscribe().with(item -> {
assertEquals(JsonValue.ValueType.NULL, item.getData().get("barSub").getValueType());
returned.set(true);
}, throwable -> Assertions.fail(throwable));

await().untilTrue(returned);
}
}

@Test
public void testUnauthorizedUserForQueryWebSocket() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.header("Authorization", "Basic ZGF2aWQ6cXdlcnR5MTIz")
.executeSingleOperationsOverWebsocket(true);
try (DynamicGraphQLClient client = clientBuilder.build()) {
Response response = client.executeSync("{ bar { message } }");
assertEquals(JsonValue.ValueType.NULL, response.getData().get("bar").getValueType());
}
}

@Test
public void testUnauthenticatedForQueryWebSocket() throws Exception {
DynamicGraphQLClientBuilder clientBuilder = DynamicGraphQLClientBuilder.newBuilder()
.url(url)
.executeSingleOperationsOverWebsocket(true);
try (DynamicGraphQLClient client = clientBuilder.build()) {
Response response = client.executeSync("{ foo { message} }");
assertEquals(JsonValue.ValueType.NULL, response.getData().get("foo").getValueType());
}
}

public static class Foo {

private String message;

public Foo(String foo) {
this.message = foo;
}

public String getMessage() {
return message;
}

public void setMessage(String message) {
this.message = message;
}

}

@GraphQLApi
public static class SecuredApi {

@Query
@RolesAllowed("fooRole")
@NonBlocking
public Foo foo() {
return new Foo("foo");
}

@Query
@RolesAllowed("barRole")
public Foo bar() {
return new Foo("bar");
}

@Subscription
@RolesAllowed("fooRole")
public Multi<Foo> fooSub() {
return Multi.createFrom().emitter(emitter -> {
emitter.emit(new Foo("foo"));
emitter.complete();
});
}

@Subscription
@RolesAllowed("barRole")
public Multi<Foo> barSub() {
return Multi.createFrom().emitter(emitter -> {
emitter.emit(new Foo("bar"));
emitter.complete();
});
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
quarkus.security.users.file.enabled=true
quarkus.security.users.file.plain-text=true
quarkus.security.users.file.users=users.properties
quarkus.security.users.file.roles=roles.properties
quarkus.http.auth.basic=true

quarkus.smallrye-graphql.log-payload=queryAndVariables
quarkus.smallrye-graphql.print-data-fetcher-exception=true
quarkus.smallrye-graphql.error-extension-fields=exception,classification,code,description,validationErrorType,queryPath

quarkus.http.auth.permission.authenticated.paths=/graphql
quarkus.http.auth.permission.authenticated.methods=GET,POST
quarkus.http.auth.permission.authenticated.policy=authenticated
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
quarkus.security.users.file.enabled=true
quarkus.security.users.file.plain-text=true
quarkus.security.users.file.users=users.properties
quarkus.security.users.file.roles=roles.properties
quarkus.http.auth.basic=true

quarkus.smallrye-graphql.log-payload=queryAndVariables
quarkus.smallrye-graphql.print-data-fetcher-exception=true
quarkus.smallrye-graphql.error-extension-fields=exception,classification,code,description,validationErrorType,queryPath
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
david=fooRole
Loading
Loading