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

Remove unused accessible? methods #4336

Merged
merged 1 commit into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions lib/graphql/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -826,10 +826,6 @@ def visible?(member, ctx)
member.visible?(ctx)
end

def accessible?(member, ctx)
member.accessible?(ctx)
end

def schema_directive(dir_class, **options)
@own_schema_directives ||= []
Member::HasDirectives.add_directive(self, @own_schema_directives, dir_class, options)
Expand All @@ -839,18 +835,6 @@ def schema_directives
Member::HasDirectives.get_directives(self, @own_schema_directives, :schema_directives)
end

# This hook is called when a client tries to access one or more
# fields that fail the `accessible?` check.
#
# By default, an error is added to the response. Override this hook to
# track metrics or return a different error to the client.
#
# @param error [InaccessibleFieldsError] The analysis error for this check
# @return [AnalysisError, nil] Return an error to skip the query
def inaccessible_fields(error)
error
end

# This hook is called when an object fails an `authorized?` check.
# You might report to your bug tracker here, so you can correct
# the field resolvers not to return unauthorized objects.
Expand Down
4 changes: 0 additions & 4 deletions lib/graphql/schema/argument.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,6 @@ def visible?(context)
true
end

def accessible?(context)
true
end

def authorized?(obj, value, ctx)
authorized_as_type?(obj, value, ctx, as_type: type)
end
Expand Down
1 change: 0 additions & 1 deletion lib/graphql/schema/enum_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ def inspect
end

def visible?(_ctx); true; end
def accessible?(_ctx); true; end
def authorized?(_ctx); true; end
end
end
Expand Down
8 changes: 0 additions & 8 deletions lib/graphql/schema/field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -591,14 +591,6 @@ def visible?(context)
end
end

def accessible?(context)
if @resolver_class
@resolver_class.accessible?(context)
else
true
end
end

def authorized?(object, args, context)
if @resolver_class
# The resolver _instance_ will check itself during `resolve()`
Expand Down
10 changes: 0 additions & 10 deletions lib/graphql/schema/interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,6 @@ def visible?(context)
true
end

# The interface is accessible if any of its possible types are accessible
def accessible?(context)
context.schema.possible_types(self, context).each do |type|
if context.schema.accessible?(type, context)
return true
end
end
false
end

def type_membership_class(membership_class = nil)
if membership_class
@type_membership_class = membership_class
Expand Down
4 changes: 0 additions & 4 deletions lib/graphql/schema/member/base_dsl_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ def visible?(context)
true
end

def accessible?(context)
true
end

def authorized?(object, context)
true
end
Expand Down
4 changes: 0 additions & 4 deletions lib/graphql/types/relay/connection_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ def authorized?(obj, ctx)
true # Let nodes be filtered out
end

def accessible?(ctx)
node_type.accessible?(ctx)
end

def visible?(ctx)
# if this is an abstract base class, there may be no `node_type`
node_type ? node_type.visible?(ctx) : super
Expand Down
4 changes: 0 additions & 4 deletions lib/graphql/types/relay/edge_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ def authorized?(obj, ctx)
true
end

def accessible?(ctx)
node_type.accessible?(ctx)
end

def visible?(ctx)
node_type.visible?(ctx)
end
Expand Down
65 changes: 2 additions & 63 deletions spec/graphql/authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ def visible?(context)
super && (context[:hide] ? @name != "hidden" : true)
end

def accessible?(context)
super && (context[:hide] ? @name != "inaccessible" : true)
end

def authorized?(parent_object, value, context)
super && parent_object != :hide2
end
Expand All @@ -45,10 +41,6 @@ def visible?(context)
super && (context[:hide] ? @name != "hidden" : true)
end

def accessible?(context)
super && (context[:hide] ? @name != "inaccessible" : true)
end

def authorized?(object, args, context)
if object == :raise
raise GraphQL::UnauthorizedFieldError.new("raised authorized field error", object: object)
Expand Down Expand Up @@ -117,51 +109,13 @@ def self.visible?(ctx)
super && !ctx[:hidden_relay]
end

def self.accessible?(ctx)
super && !ctx[:inaccessible_relay]
end

def self.authorized?(_val, ctx)
super && !ctx[:unauthorized_relay]
end

field :some_field, String
end

# TODO test default behavior for abstract types,
# that they check their concrete types
module InaccessibleInterface
include BaseInterface

def self.accessible?(ctx)
super && !ctx[:hide]
end

def self.resolve_type(obj, ctx)
InaccessibleObject
end
end

module InaccessibleDefaultInterface
include BaseInterface
# accessible? will call the super method
def self.resolve_type(obj, ctx)
InaccessibleObject
end

field :some_field, String
end

class InaccessibleObject < BaseObject
implements InaccessibleInterface
implements InaccessibleDefaultInterface
def self.accessible?(ctx)
super && !ctx[:hide]
end

field :some_field, String
end

class UnauthorizedObject < BaseObject
def self.authorized?(value, context)
if context[:raise]
Expand Down Expand Up @@ -251,7 +205,7 @@ def self.authorized?(obj, ctx)
class LandscapeFeature < BaseEnum
value "MOUNTAIN"
value "STREAM", role: :unauthorized
value "FIELD", role: :inaccessible
value "FIELD"
value "TAR_PIT", role: :hidden
end

Expand All @@ -265,7 +219,6 @@ def self.authorized?(obj, ctx)
field :int2, Integer do
argument :int, Integer, required: false
argument :hidden, Integer, required: false
argument :inaccessible, Integer, required: false
argument :unauthorized, Integer, required: false
end

Expand Down Expand Up @@ -298,13 +251,6 @@ def empty_array; []; end
field :hidden_connection, RelayObject.connection_type, null: :false, resolver_method: :empty_array
field :hidden_edge, RelayObject.edge_type, null: :false, resolver_method: :edge_object

field :inaccessible, Integer, null: false, method: :object_id
field :inaccessible_object, InaccessibleObject, null: false, resolver_method: :itself
field :inaccessible_interface, InaccessibleInterface, null: false, resolver_method: :itself
field :inaccessible_default_interface, InaccessibleDefaultInterface, null: false, resolver_method: :itself
field :inaccessible_connection, RelayObject.connection_type, null: :false, resolver_method: :empty_array
field :inaccessible_edge, RelayObject.edge_type, null: :false, resolver_method: :edge_object

field :unauthorized_object, UnauthorizedObject, resolver_method: :itself
field :unauthorized_connection, RelayObject.connection_type, null: false, resolver_method: :array_with_item
field :unauthorized_edge, RelayObject.edge_type, null: false, resolver_method: :edge_object
Expand Down Expand Up @@ -375,12 +321,6 @@ def self.visible?(ctx)
field :some_return_field, String
end

class DoInaccessibleStuff < GraphQL::Schema::RelayClassicMutation
def self.accessible?(ctx)
super && (ctx[:inaccessible_mutation] ? false : true)
end
end

class DoUnauthorizedStuff < GraphQL::Schema::RelayClassicMutation
def self.authorized?(obj, ctx)
super && (ctx[:unauthorized_mutation] ? false : true)
Expand All @@ -390,7 +330,6 @@ def self.authorized?(obj, ctx)
class Mutation < BaseObject
field :do_hidden_stuff, mutation: DoHiddenStuff
field :do_hidden_stuff2, mutation: DoHiddenStuff2
field :do_inaccessible_stuff, mutation: DoInaccessibleStuff
field :do_unauthorized_stuff, mutation: DoUnauthorizedStuff
end

Expand Down Expand Up @@ -584,7 +523,7 @@ def auth_execute(*args, **kwargs)
query_field_names = res["data"]["query"]["fields"].map { |f| f["name"] }
refute_includes query_field_names, "int"
int2_arg_names = res["data"]["query"]["fields"].find { |f| f["name"] == "int2" }["args"].map { |a| a["name"] }
assert_equal ["int", "inaccessible", "unauthorized"], int2_arg_names
assert_equal ["int", "unauthorized"], int2_arg_names

assert_nil res["data"]["hiddenObject"]
assert_nil res["data"]["hiddenInterface"]
Expand Down