exAspArk / graphql-guard

Simple authorization gem for GraphQL :lock:
MIT License
471 stars 36 forks source link

GraphQL::Guard::NotAuthorizedError: 500 instead of 401. How to rescue? #8

Closed ghost closed 6 years ago

ghost commented 6 years ago

This is NOT a bug and I immediately apologize for this issue but I'm newbie in Ruby, Rails and this amazing gem.

When I get this: GraphQL::Guard::NotAuthorizedError:

{
    "status": 500,
    "error": "Internal Server Error",
    "exception": "#<GraphQL::Guard::NotAuthorizedError: Query.users>",
    "traces": {
        "Application Trace": [
            {
                "id": 35,
                "trace": "app/controllers/graphql_controller.rb:11:in `execute'"
            }
        ],
        "Framework Trace": [
            {
                "id": 0,
                "trace": "graphql-guard (1.0.0) lib/graphql/guard.rb:12:in `block in <class:Guard>'"
            },
            {
                "id": 1,
                "trace": "graphql-guard (1.0.0) lib/graphql/guard.rb:38:in `block in instrument'"
            },
            {
                "id": 2,
                "trace": "graphql (1.7.6) lib/graphql/field.rb:230:in `resolve'"
            },
            {
                "id": 3,
                "trace": "graphql (1.7.6) lib/graphql/execution/execute.rb:254:in `call'"
            },
            {
                "id": 4,
                "trace": "graphql (1.7.6) lib/graphql/schema/middleware_chain.rb:45:in `invoke_core'"
            },
            {
                "id": 5,
                "trace": "graphql (1.7.6) lib/graphql/schema/middleware_chain.rb:38:in `invoke'"
            }, ....................

I would prefer something like REST API Unauthorized error 401.

How to do that?

I have to rescue that exception?

Something like:

render json: ["Error 401. Unauthorized."], status: :unauthorized

exAspArk commented 6 years ago

Hey @johnunclesam!

I would prefer something like REST API Unauthorized error 401. How to do that?

That's a good question. Unfortunately, currently, it's not possible with the gem out of the box.

One of the reasons is because GraphQL as a specification doesn't care about the transport level (HTTP). That is why when you get a query error (like syntax or type), GraphQL by default will return 200 OK and the array of errors in the response itself. So, it's very common to always return 200. That's why I even created the gem https://github.com/exAspArk/graphql-errors.

If you still would like to return custom HTTP status code, yes, you can raise an exception and rescue it in the controller:

MySchema = GraphQL::Schema.define do
  query QueryType
  use GraphQL::Guard.new(
    not_authorized: ->(type, field) { raise MyException }
  )
end

# controller
...
rescue MyException 
  render json: ..., status: :unauthorized
end
exAspArk commented 6 years ago

@johnunclesam you can also follow the discussion https://github.com/rmosolgo/graphql-ruby/issues/1130 in case if somebody will reply :)

ghost commented 6 years ago

Thanks for your answer.

Your link to that discussion got me to this: https://www.graph.cool/docs/faq/api-eep0ugh1wa/#how-does-error-handling-work-with-graphcool

and this:

Notice that HTTP status codes are not relevant when using GraphQL! A GraphQL server will always respond with code 200 (unless an internal server occured, then it might be code 500). Any further information needs to be parsed from the JSON payload of the response.

So the question is: with grahql-guard I receive 500 on NotAuthorized like this:

{
    "status": 500,
    "error": "Internal Server Error",
    "exception": "#<GraphQL::Guard::NotAuthorizedError: Query.players>",
    "traces": {
        "Application Trace": [
            {
                "id": 35,
                "trace": "app/controllers/graphql_controller.rb:10:in `execute'"
            }
        ],
        "Framework Trace": [
            {
                "id": 0,
                "trace": "graphql-guard (1.0.0) lib/graphql/guard.rb:12:in `block in <class:Guard>'"
            },
            {
                "id": 1,
                "trace": "graphql-guard (1.0.0) lib/graphql/guard.rb:38:in `block in instrument'"
            },

is it correct?

Isn't better to response with 200 and error: "You are not authorized" as the graphql specs says?

exAspArk commented 6 years ago

Yes, if you would like to return 200:

SchemaWithErrors = GraphQL::Schema.define do
  query QueryType
  use GraphQL::Guard.new(
    # Returns an error in the response
    not_authorized: ->(type, field) { GraphQL::ExecutionError.new("Not authorized to access #{type}.#{field}") }
  )
end

https://github.com/exAspArk/graphql-guard#error-handling

Which can become a default behavior in the future (don't want to introduce breaking changes for now).

ghost commented 6 years ago

I vote for default behavior. Also from now. Thanks a lot.