Skip to content

Commit

Permalink
Merge pull request #4336 from rmosolgo/remove-unused-accessible-methods
Browse files Browse the repository at this point in the history
Remove unused accessible? methods
  • Loading branch information
rmosolgo authored Feb 8, 2023
2 parents 66b005c + a2a6f82 commit 36bff53
Show file tree
Hide file tree
Showing 9 changed files with 2 additions and 114 deletions.
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

0 comments on commit 36bff53

Please sign in to comment.