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

Subscriptions: Unexpected behavior when returning false for authorized? #3390

Closed
nixme opened this issue Mar 13, 2021 · 2 comments · Fixed by #3662
Closed

Subscriptions: Unexpected behavior when returning false for authorized? #3390

nixme opened this issue Mar 13, 2021 · 2 comments · Fixed by #3662

Comments

@nixme
Copy link

nixme commented Mar 13, 2021

Describe the bug

The documentation for authorizing subscriptions says the authorized? method...

"may return false or raise a GraphQL::ExecutionError to halt execution"

However, while returning false does ensure the payload is nil, the subscription is still registered via write_subscription. Every subsequent trigger to the topic will send these unauthorized subscriptions an empty payload. No data leaks, but unauthorized subscribers still know something happened.

Additionally, the Authorization docs state this behavior can be customized by implementing Schema.unauthorized_object, however that doesn't to get called for subscriptions. (Perhaps related to #2048?)

Versions

graphql: 1.12.5
graphql-pro: 1.17.8
rails: 5.2

GraphQL schema

Example:

class UpdateType < GraphQL::Schema::Object
  field :message, String, null: true
end

class Updates < GraphQL::Schema::Subscription
  payload_type UpdateType

  def authorized?(**)
    false
  end
end

class MySchema < GraphQL::Schema
  class SubscriptionType < GraphQL::Schema::Object
    field :updates, subscription: Updates, null: true
  end

  use GraphQL::Subscriptions::ActionCableSubscriptions
  subscription SubscriptionType

  def self.unauthorized_object(error)
    raise GraphQL::ExecutionError, "Unauthorized access"   # <== This never gets called
  end

  def self.unauthorized_field(error)
    raise GraphQL::ExecutionError, "Unauthorized access"
  end
end

GraphQL query

subscription Updates {
  updates {
    message
  }
}

Expected behavior

@rmosolgo
Copy link
Owner

Thanks for the detailed bug report. I agree, the behavior you expect here is how it should work. I'll take a look soon!

@tienle
Copy link

tienle commented Jun 7, 2021

Agree, I also find this behaviour is confused.

module Subscriptions
  class MessageAdded < BaseSubscription
    description 'A message was added'
    argument :channel_id, ID, required: true

    field :message, Types::ChatMessageType, null: true
    
    def authorized?(channel_id:)
       false
    end

Got error response from subscription:
GraphqlChannel transmitting {:result=>{"data"=>nil, "errors"=>[{"message"=>"Cannot return null for non-nullable field Subscription.messageAdded"}]}, :more=>true}

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

Successfully merging a pull request may close this issue.

3 participants