diff options
Diffstat (limited to 'spec/lib/gitlab/graphql')
-rw-r--r-- | spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb | 226 | ||||
-rw-r--r-- | spec/lib/gitlab/graphql/lazy_spec.rb | 96 | ||||
-rw-r--r-- | spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb | 23 |
3 files changed, 266 insertions, 79 deletions
diff --git a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb index 7576523ce52..c88506899cd 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb @@ -27,13 +27,17 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do end end + def resolve + service.authorized_resolve[type_instance, {}, context] + end + subject(:service) { described_class.new(field) } describe '#authorized_resolve' do let_it_be(:current_user) { build(:user) } let_it_be(:presented_object) { 'presented object' } let_it_be(:query_type) { GraphQL::ObjectType.new } - let_it_be(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)} + let_it_be(:schema) { GitlabSchema } let_it_be(:query) { GraphQL::Query.new(schema, document: nil, context: {}, variables: {}) } let_it_be(:context) { GraphQL::Query::Context.new(query: query, values: { current_user: current_user }, object: nil) } @@ -41,125 +45,201 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do let(:type_instance) { type_class.authorized_new(presented_object, context) } let(:field) { type_class.fields['testField'].to_graphql } - subject(:resolved) { service.authorized_resolve.call(type_instance, {}, context) } + subject(:resolved) { ::Gitlab::Graphql::Lazy.force(resolve) } - context 'scalar types' do - shared_examples 'checking permissions on the presented object' do - it 'checks the abilities on the object being presented and returns the value' do - expected_permissions.each do |permission| - spy_ability_check_for(permission, presented_object, passed: true) - end + context 'reading the field of a lazy value' do + let(:ability) { :read_field } + let(:presented_object) { lazy_upcase('a') } + let(:type_class) { type_with_field(GraphQL::STRING_TYPE, ability) } - expect(resolved).to eq('Resolved value') + let(:upcaser) do + Module.new do + def self.upcase(strs) + strs.map(&:upcase) + end end + end - it 'returns nil if the value was not authorized' do - allow(Ability).to receive(:allowed?).and_return false - - expect(resolved).to be_nil + def lazy_upcase(str) + ::BatchLoader::GraphQL.for(str).batch do |strs, found| + strs.zip(upcaser.upcase(strs)).each { |s, us| found[s, us] } end end - context 'when the field is a built-in scalar type' do - let(:type_class) { type_with_field(GraphQL::STRING_TYPE, :read_field) } - let(:expected_permissions) { [:read_field] } + it 'does not run authorizations until we force the resolved value' do + expect(Ability).not_to receive(:allowed?) - it_behaves_like 'checking permissions on the presented object' + expect(resolve).to respond_to(:force) end - context 'when the field is a list of scalar types' do - let(:type_class) { type_with_field([GraphQL::STRING_TYPE], :read_field) } - let(:expected_permissions) { [:read_field] } + it 'runs authorizations when we force the resolved value' do + spy_ability_check_for(ability, 'A') - it_behaves_like 'checking permissions on the presented object' + expect(resolved).to eq('Resolved value') end - context 'when the field is sub-classed scalar type' do - let(:type_class) { type_with_field(Types::TimeType, :read_field) } - let(:expected_permissions) { [:read_field] } + it 'redacts values that fail the permissions check' do + spy_ability_check_for(ability, 'A', passed: false) - it_behaves_like 'checking permissions on the presented object' + expect(resolved).to be_nil end - context 'when the field is a list of sub-classed scalar types' do - let(:type_class) { type_with_field([Types::TimeType], :read_field) } - let(:expected_permissions) { [:read_field] } + context 'we batch two calls' do + def resolve(value) + instance = type_class.authorized_new(lazy_upcase(value), context) + service.authorized_resolve[instance, {}, context] + end - it_behaves_like 'checking permissions on the presented object' - end - end + it 'batches resolution, but authorizes each object separately' do + expect(upcaser).to receive(:upcase).once.and_call_original + spy_ability_check_for(:read_field, 'A', passed: true) + spy_ability_check_for(:read_field, 'B', passed: false) + spy_ability_check_for(:read_field, 'C', passed: true) - context 'when the field is a connection' do - context 'when it resolves to nil' do - let(:type_class) { type_with_field(Types::QueryType.connection_type, :read_field, nil) } + a = resolve('a') + b = resolve('b') + c = resolve('c') - it 'does not fail when authorizing' do - expect(resolved).to be_nil + expect(a.force).to be_present + expect(b.force).to be_nil + expect(c.force).to be_present end end end - context 'when the field is a specific type' do - let(:custom_type) { type(:read_type) } - let(:object_in_field) { double('presented in field') } + shared_examples 'authorizing fields' do + context 'scalar types' do + shared_examples 'checking permissions on the presented object' do + it 'checks the abilities on the object being presented and returns the value' do + expected_permissions.each do |permission| + spy_ability_check_for(permission, presented_object, passed: true) + end - let(:type_class) { type_with_field(custom_type, :read_field, object_in_field) } - let(:type_instance) { type_class.authorized_new(object_in_field, context) } + expect(resolved).to eq('Resolved value') + end - subject(:resolved) { service.authorized_resolve.call(type_instance, {}, context) } + it 'returns nil if the value was not authorized' do + allow(Ability).to receive(:allowed?).and_return false - it 'checks both field & type permissions' do - spy_ability_check_for(:read_field, object_in_field, passed: true) - spy_ability_check_for(:read_type, object_in_field, passed: true) + expect(resolved).to be_nil + end + end - expect(resolved).to eq(object_in_field) - end + context 'when the field is a built-in scalar type' do + let(:type_class) { type_with_field(GraphQL::STRING_TYPE, :read_field) } + let(:expected_permissions) { [:read_field] } - it 'returns nil if viewing was not allowed' do - spy_ability_check_for(:read_field, object_in_field, passed: false) - spy_ability_check_for(:read_type, object_in_field, passed: true) + it_behaves_like 'checking permissions on the presented object' + end - expect(resolved).to be_nil + context 'when the field is a list of scalar types' do + let(:type_class) { type_with_field([GraphQL::STRING_TYPE], :read_field) } + let(:expected_permissions) { [:read_field] } + + it_behaves_like 'checking permissions on the presented object' + end + + context 'when the field is sub-classed scalar type' do + let(:type_class) { type_with_field(Types::TimeType, :read_field) } + let(:expected_permissions) { [:read_field] } + + it_behaves_like 'checking permissions on the presented object' + end + + context 'when the field is a list of sub-classed scalar types' do + let(:type_class) { type_with_field([Types::TimeType], :read_field) } + let(:expected_permissions) { [:read_field] } + + it_behaves_like 'checking permissions on the presented object' + end end - context 'when the field is not nullable' do - let(:type_class) { type_with_field(custom_type, :read_field, object_in_field, null: false) } + context 'when the field is a connection' do + context 'when it resolves to nil' do + let(:type_class) { type_with_field(Types::QueryType.connection_type, :read_field, nil) } + + it 'does not fail when authorizing' do + expect(resolved).to be_nil + end + end - it 'returns nil when viewing is not allowed' do - spy_ability_check_for(:read_type, object_in_field, passed: false) + context 'when it returns values' do + let(:objects) { [1, 2, 3] } + let(:field_type) { type([:read_object]).connection_type } + let(:type_class) { type_with_field(field_type, [], objects) } - expect(resolved).to be_nil + it 'filters out unauthorized values' do + spy_ability_check_for(:read_object, 1, passed: true) + spy_ability_check_for(:read_object, 2, passed: false) + spy_ability_check_for(:read_object, 3, passed: true) + + expect(resolved.nodes).to eq [1, 3] + end end end - context 'when the field is a list' do - let(:object_1) { double('presented in field 1') } - let(:object_2) { double('presented in field 2') } - let(:presented_types) { [double(object: object_1), double(object: object_2)] } + context 'when the field is a specific type' do + let(:custom_type) { type(:read_type) } + let(:object_in_field) { double('presented in field') } + + let(:type_class) { type_with_field(custom_type, :read_field, object_in_field) } + let(:type_instance) { type_class.authorized_new(object_in_field, context) } + + it 'checks both field & type permissions' do + spy_ability_check_for(:read_field, object_in_field, passed: true) + spy_ability_check_for(:read_type, object_in_field, passed: true) + + expect(resolved).to eq(object_in_field) + end + + it 'returns nil if viewing was not allowed' do + spy_ability_check_for(:read_field, object_in_field, passed: false) + spy_ability_check_for(:read_type, object_in_field, passed: true) - let(:type_class) { type_with_field([custom_type], :read_field, presented_types) } - let(:type_instance) { type_class.authorized_new(presented_types, context) } + expect(resolved).to be_nil + end - it 'checks all permissions' do - allow(Ability).to receive(:allowed?) { true } + context 'when the field is not nullable' do + let(:type_class) { type_with_field(custom_type, :read_field, object_in_field, null: false) } - spy_ability_check_for(:read_field, object_1, passed: true) - spy_ability_check_for(:read_type, object_1, passed: true) - spy_ability_check_for(:read_field, object_2, passed: true) - spy_ability_check_for(:read_type, object_2, passed: true) + it 'returns nil when viewing is not allowed' do + spy_ability_check_for(:read_type, object_in_field, passed: false) - expect(resolved).to eq(presented_types) + expect(resolved).to be_nil + end end - it 'filters out objects that the user cannot see' do - allow(Ability).to receive(:allowed?) { true } + context 'when the field is a list' do + let(:object_1) { double('presented in field 1') } + let(:object_2) { double('presented in field 2') } + let(:presented_types) { [double(object: object_1), double(object: object_2)] } + + let(:type_class) { type_with_field([custom_type], :read_field, presented_types) } + let(:type_instance) { type_class.authorized_new(presented_types, context) } + + it 'checks all permissions' do + allow(Ability).to receive(:allowed?) { true } - spy_ability_check_for(:read_type, object_1, passed: false) + spy_ability_check_for(:read_field, object_1, passed: true) + spy_ability_check_for(:read_type, object_1, passed: true) + spy_ability_check_for(:read_field, object_2, passed: true) + spy_ability_check_for(:read_type, object_2, passed: true) - expect(resolved.map(&:object)).to contain_exactly(object_2) + expect(resolved).to eq(presented_types) + end + + it 'filters out objects that the user cannot see' do + allow(Ability).to receive(:allowed?) { true } + + spy_ability_check_for(:read_type, object_1, passed: false) + + expect(resolved).to contain_exactly(have_attributes(object: object_2)) + end end end end + + it_behaves_like 'authorizing fields' end private diff --git a/spec/lib/gitlab/graphql/lazy_spec.rb b/spec/lib/gitlab/graphql/lazy_spec.rb new file mode 100644 index 00000000000..795978ab0a4 --- /dev/null +++ b/spec/lib/gitlab/graphql/lazy_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Graphql::Lazy do + def load(key) + BatchLoader.for(key).batch do |keys, loader| + keys.each { |x| loader.call(x, x * x) } + end + end + + let(:value) { double(x: 1) } + + describe '#force' do + subject { described_class.new { value.x } } + + it 'can extract the value' do + expect(subject.force).to be 1 + end + + it 'can derive new lazy values' do + expect(subject.then { |x| x + 2 }.force).to be 3 + end + + it 'only evaluates once' do + expect(value).to receive(:x).once + + expect(subject.force).to eq(subject.force) + end + + it 'deals with nested laziness' do + expect(described_class.new { load(10) }.force).to eq(100) + expect(described_class.new { described_class.new { 5 } }.force).to eq 5 + end + end + + describe '.with_value' do + let(:inner) { described_class.new { value.x } } + + subject { described_class.with_value(inner) { |x| x.to_s } } + + it 'defers the application of a block to a value' do + expect(value).not_to receive(:x) + + expect(subject).to be_an_instance_of(described_class) + end + + it 'evaluates to the application of the block to the value' do + expect(value).to receive(:x).once + + expect(subject.force).to eq(inner.force.to_s) + end + end + + describe '.force' do + context 'when given a plain value' do + subject { described_class.force(1) } + + it 'unwraps the value' do + expect(subject).to be 1 + end + end + + context 'when given a wrapped lazy value' do + subject { described_class.force(described_class.new { 2 }) } + + it 'unwraps the value' do + expect(subject).to be 2 + end + end + + context 'when the value is from a batchloader' do + subject { described_class.force(load(3)) } + + it 'syncs the value' do + expect(subject).to be 9 + end + end + + context 'when the value is a GraphQL lazy' do + subject { described_class.force(GitlabSchema.after_lazy(load(3)) { |x| x + 1 } ) } + + it 'forces the evaluation' do + expect(subject).to be 10 + end + end + + context 'when the value is a promise' do + subject { described_class.force(::Concurrent::Promise.new { 4 }) } + + it 'executes the promise and waits for the value' do + expect(subject).to be 4 + end + end + end +end diff --git a/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb index cf1f00bc176..7ae33346388 100644 --- a/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb +++ b/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' RSpec.describe Gitlab::Graphql::Loaders::BatchModelLoader do describe '#find' do - let(:issue) { create(:issue) } - let(:user) { create(:user) } + let_it_be(:issue) { create(:issue) } + let_it_be(:other_user) { create(:user) } + let_it_be(:user) { create(:user) } it 'finds a model by id' do issue_result = described_class.new(Issue, issue.id).find @@ -16,15 +17,25 @@ RSpec.describe Gitlab::Graphql::Loaders::BatchModelLoader do end it 'only queries once per model' do - other_user = create(:user) - user - issue - expect do [described_class.new(User, other_user.id).find, described_class.new(User, user.id).find, described_class.new(Issue, issue.id).find].map(&:sync) end.not_to exceed_query_limit(2) end + + it 'does not force values unnecessarily' do + expect do + a = described_class.new(User, user.id).find + b = described_class.new(Issue, issue.id).find + + b.sync + + c = described_class.new(User, other_user.id).find + + a.sync + c.sync + end.not_to exceed_query_limit(2) + end end end |