exAspArk / graphql-guard

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

Add support for context to not_authorized callback when using Policy #45

Open parse opened 4 years ago

parse commented 4 years ago

This is a question as well as a suggestion.

I'm combining graphql-ruby and graphql-guard with Doorkeeper+Sorcery to handle my authentication. In my graphql_controller.rb I have:

def current_user
    token = Doorkeeper.authenticate(request)
    unless token&.accessible?
      raise UnauthenticatedError
    end

    current_resource_owner
  rescue UnauthenticatedError => e
    GraphQL::ExecutionError.new('Unauthenticated', extensions: { code: 'AUTHENTICATION_ERROR' })
end

And my policy is:

...
Types::WorkoutType => {
    '*': ->(obj, args, ctx) { obj.try(:author) == ctx[:current_user] || ctx[:current_user].try(:admin) }
},
...

So when a user is not authenticated, I expect the Unauthenticated error to be returned, but instead I get the Not authorized to access #{type}.#{field} defined in graphl_controller.rb.

Question:

Would it make sense to add the context to the callback so one could do something like this?

use GraphQL::Guard.new(
    policy_object: GraphqlPolicy,
    not_authorized: ->(type, field, ctx) do
      ctx.add_error(GraphQL::ExecutionError.new("Not authorized to access #{type}.#{field}"))
    end
)

That way we wouldn't remove any other errors that are in there and we can see that we are in fact unauthenticated as well as unauthorized.

ghost commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

parse commented 3 years ago

Bump, would love some feedback on this. If it seems like a good idea I can give the implementation a shot.

MarinaMurashev commented 3 years ago

I am in need of this feature as well. I need to be able to tell the difference between a user trying to access a field they're not authorized to access vs the user is not logged in at all.