Skip to content

Authorization/error handling #30

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

Closed
konrad-garus opened this issue Jun 8, 2017 · 5 comments
Closed

Authorization/error handling #30

konrad-garus opened this issue Jun 8, 2017 · 5 comments

Comments

@konrad-garus
Copy link

I'm looking for a way to authorize requests, specifically mutations. For example, if a user without the required permission attempts to execute mutateFoo, they should be rejected with an error. Or maybe they do not have the permission to mutate this particular instance of Foo (so it depends on payload, not just mutation type). Offhand I would expect HTTP 403, unless the graphql-land wants to reinvent HTTP and put that in the payload.

Anyway, I don't really see a way to do this with GraphQLServlet at all. There doesn't seem to be a pre-execution hook that could veto execution and let me return an error. When an exception is thrown, it's always caught and translated to the generic:

{"data":{"mutateFoo":null},"errors":[{"message":"Internal Server Error(s) while executing query"}]}

Ideally, I should be able to throw an exception from anywhere in my business logic, and have it translated to a meaningful GraphQL response. That's even more flexible than the abovementioned execution hook.

I don't want to use something like servlet filters for this, since that would mean interpreting the GraphQL request myself, and it's on the wrong level.

@apottere
Copy link
Contributor

Good idea, I'll keep this in mind when I revamp the error handling code.

@konrad-garus
Copy link
Author

Thanks!

@adrian-baker
Copy link

Unfortunately even with 7923cdb I can't see a way to control the response code, for example to return a 403 on a permissions error.

@adrian-baker
Copy link

I had hoped I could use GraphQLServletListener, however it's difficult to get an exception bubbled up to the callback's onError method. Even if I add a custom error handler that throws the exception, it's still caught in the postHandler's try...catch block, and never gets to the onError callback.

            } catch (Exception e) {
                log.info("Bad POST request: parsing failed", e);
                response.setStatus(STATUS_BAD_REQUEST);
            }

Perhaps I'm missing a more obvious way to control the response, but it looks like I'll have to create my own implementation of AbstractGraphQLHttpServlet to do it.

@oliemansm
Copy link
Member

@adrian-baker Sounds like you're looking for something similar as requested in in #129. The default error handler is used when handling GraphQL requests and an error related to that data occurs. It sounds like you want to verify whether a user is allowed to use the GraphQL at all and return a 403 then, which would be a valid use case. In our the split modules branch where we're refactoring certain parts this will be taken into account to provide an option to customize that response.

I wouldn't advise you to change the response code if you want to verify permissions within a query however, since that goes against the GraphQL principles. Because you can run multiple queries in one request. Query #1 might be allowed, and query #2 might throw a permissions error. That's why there's the response object as it is now with data and errors so you can still get the data for the query that was valid and receive the permission related errors for the query that was forbidden. For that use case you could add your own CustomGraphQLErrorHandler to provide error codes or something else in the error responses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants