Graphql endpoint secured with RolesAllowed returns status 200 despite ForbiddenException being thrown

[adjiandov]

Describe the bug

Setup:
Quarkus with SmallRye GraphQL, OIDC with Keycloak.
When securing the endpoints with @RolesAllowed if the user(token) does not have the required roles a ForbiddenException is thrown but the request comes back with status 200 and DataFetchingError instead of 401 or 403.

Expected behavior

Response has proper message and error code.

Actual behavior

Response comes back with status 200 and DataFetching exception

How to Reproduce?

Steps to reproduce:

1. Create Quarkus application
2. Add Graphql dependency
3. Add quarkus-oidc dependency
4. Add quarkus-keycloak-authorization dependency
5. Create GraphQlEndpoint
6. Annotate the endpoint with some role
7. Create user in keycloak that does not have that role.
8. Query this endpoind.

Output of uname -a or ver

No response

Output of java -version

17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

1.8.0.Final

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

No response

Additional information

No response

[quarkus-bot[bot]]

/cc @jmartisk, @phillip-kruger

[phillip-kruger]

This is the correct behavior. We follow the GraphQL over HTTP Spec (see https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md)

GraphQL will return 200 with the errors in the error fields. GraphQL is not tightly coupled with HTTP.

In Quarkus you might want to set quarkus.smallrye-graphql.error-extension-fields (see https://quarkus.io/guides/smallrye-graphql#quarkus-smallrye-graphql_quarkus.smallrye-graphql.error-extension-fields) to get more error details

I am going to move this to a discussion.

[adjiandov]

Ok, but the specification says the following:

A server MAY forbid individual requests by a client to any endpoint for any reason, for example to require authentication or payment; when doing so it SHOULD use the relevant 4xx or 5xx status code. This decision SHOULD NOT be based on the contents of a well formed GraphQL request.

[phillip-kruger]

We already decided to stick to 200 with errors in the error field. We need to be consistent. At best we can consider a config option to switch between the two models, but I am not sure it's worth it. If anyone else want to contribute something like that we can look at they option

[phillip-kruger]

Forbidden means you are authorised, but do not have the rights, unauthorised means you are not authorised

[adjiandov]

I know what it means, I am wandering why is the identity Anonymous when the RolesAllowedCheck is performed. Which property configures the identity type?

[adjiandov]

Hmm. Just thinking: If we went for this, how would we differentiate between requests that are completely forbidden and requests where only a part of the response is "forbidden", and some other data is present? You can't do a non-200 code if there's some data present. And it seems weird that a partial "forbidden" would be 200, while a full "forbidden" would be 403.

@jmartisk Btw I tested this scenario and it seems that if one part of the query is forbidden it returns null for data and the forbidden error in the errors. So it seems like it is treating it as full-forbidden anyways

[phillip-kruger]

No we do support partial response when using source fields