Commit 58931f4a authored by Alex Kalderimis's avatar Alex Kalderimis

Behavioral changes: Return nil instead of raising errors

This changes some of our GraphQL resolution behavior to return
nil when values cannot be authorized. This is the correct behavior,
and prevents resource scanning.
parent 9c2b4578
...@@ -3,9 +3,13 @@ ...@@ -3,9 +3,13 @@
module Resolvers module Resolvers
class BoardListsResolver < BaseResolver class BoardListsResolver < BaseResolver
include BoardIssueFilterable include BoardIssueFilterable
prepend ManualAuthorization
include Gitlab::Graphql::Authorize::AuthorizeResource include Gitlab::Graphql::Authorize::AuthorizeResource
type Types::BoardListType, null: true type Types::BoardListType, null: true
extras [:lookahead]
authorize :read_list
argument :id, Types::GlobalIDType[List], argument :id, Types::GlobalIDType[List],
required: false, required: false,
...@@ -42,10 +46,6 @@ module Resolvers ...@@ -42,10 +46,6 @@ module Resolvers
service.execute(board, create_default_lists: false) service.execute(board, create_default_lists: false)
end end
def authorized_resource?(board)
Ability.allowed?(context[:current_user], :read_list, board)
end
def load_preferences?(lookahead) def load_preferences?(lookahead)
lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed) lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed)
end end
......
# frozen_string_literal: true
# TODO: remove this entirely when framework authorization is released
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/290216
module ManualAuthorization
def resolve(**args)
super
rescue ::Gitlab::Graphql::Errors::ResourceNotAvailable
nil
end
end
...@@ -3,10 +3,11 @@ ...@@ -3,10 +3,11 @@
module Resolvers module Resolvers
module Projects module Projects
class JiraImportsResolver < BaseResolver class JiraImportsResolver < BaseResolver
type Types::JiraImportType.connection_type, null: true prepend ::ManualAuthorization
include Gitlab::Graphql::Authorize::AuthorizeResource include Gitlab::Graphql::Authorize::AuthorizeResource
type Types::JiraImportType.connection_type, null: true
alias_method :project, :object alias_method :project, :object
def resolve(**args) def resolve(**args)
......
...@@ -3,9 +3,11 @@ ...@@ -3,9 +3,11 @@
module Resolvers module Resolvers
module Projects module Projects
class ServicesResolver < BaseResolver class ServicesResolver < BaseResolver
prepend ManualAuthorization
include Gitlab::Graphql::Authorize::AuthorizeResource include Gitlab::Graphql::Authorize::AuthorizeResource
type Types::Projects::ServiceType.connection_type, null: true type Types::Projects::ServiceType.connection_type, null: true
authorize :admin_project
argument :active, argument :active,
GraphQL::BOOLEAN_TYPE, GraphQL::BOOLEAN_TYPE,
...@@ -24,10 +26,6 @@ module Resolvers ...@@ -24,10 +26,6 @@ module Resolvers
services(args[:active], args[:type]) services(args[:active], args[:type])
end end
def authorized_resource?(project)
Ability.allowed?(context[:current_user], :admin_project, project)
end
private private
def services(active, type) def services(active, type)
......
...@@ -3,9 +3,11 @@ ...@@ -3,9 +3,11 @@
module Resolvers module Resolvers
module Snippets module Snippets
class BlobsResolver < BaseResolver class BlobsResolver < BaseResolver
prepend ManualAuthorization
include Gitlab::Graphql::Authorize::AuthorizeResource include Gitlab::Graphql::Authorize::AuthorizeResource
type Types::Snippets::BlobType.connection_type, null: true type Types::Snippets::BlobType.connection_type, null: true
authorize :read_snippet
alias_method :snippet, :object alias_method :snippet, :object
...@@ -27,10 +29,6 @@ module Resolvers ...@@ -27,10 +29,6 @@ module Resolvers
end end
end end
def authorized_resource?(snippet)
Ability.allowed?(context[:current_user], :read_snippet, snippet)
end
private private
def transformed_blob_paths(paths) def transformed_blob_paths(paths)
......
...@@ -34,6 +34,8 @@ module Mutations ...@@ -34,6 +34,8 @@ module Mutations
def find_dast_site_profile(project:, global_id:) def find_dast_site_profile(project:, global_id:)
project.dast_site_profiles.find(global_id.model_id) project.dast_site_profiles.find(global_id.model_id)
rescue ActiveRecord::RecordNotFound
raise_resource_not_available_error!
end end
end end
end end
......
...@@ -24,17 +24,7 @@ RSpec.describe 'Creating a DAST Site Profile' do ...@@ -24,17 +24,7 @@ RSpec.describe 'Creating a DAST Site Profile' do
it 'returns the dast_site_profile id' do it 'returns the dast_site_profile id' do
subject subject
expect(mutation_response["id"]).to eq(dast_site_profile.to_global_id.to_s) expect(mutation_response).to include('id' => global_id_of(dast_site_profile))
end
context 'when an unknown error occurs' do
before do
allow(DastSiteProfile).to receive(:create!).and_raise(StandardError)
end
it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { contain_exactly(include('Internal server error')) }
end
end end
end end
end end
...@@ -38,9 +38,8 @@ RSpec.describe 'Creating a DAST Site Profile' do ...@@ -38,9 +38,8 @@ RSpec.describe 'Creating a DAST Site Profile' do
dast_site_profile.destroy! dast_site_profile.destroy!
end end
it_behaves_like 'a mutation that returns top-level errors' do it_behaves_like 'a mutation that returns top-level errors',
let(:match_errors) { contain_exactly(include("Internal server error: Couldn't find DastSiteProfile")) } errors: [::Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
end
end end
context 'when wrong type of global id is passed' do context 'when wrong type of global id is passed' do
......
...@@ -73,18 +73,6 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do ...@@ -73,18 +73,6 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do
it_behaves_like 'raises a resource not available error' it_behaves_like 'raises a resource not available error'
end end
context 'when user cannot access board' do
let(:board) { create(:board, group: create(:group, :private)) }
it_behaves_like 'raises a resource not available error'
end
context 'when passing board_id as nil' do
let(:board) { nil }
it_behaves_like 'raises a resource not available error'
end
end end
end end
end end
...@@ -29,9 +29,7 @@ RSpec.describe Resolvers::BoardListsResolver do ...@@ -29,9 +29,7 @@ RSpec.describe Resolvers::BoardListsResolver do
context 'with unauthorized user' do context 'with unauthorized user' do
it 'raises an error' do it 'raises an error' do
expect do expect(resolve_board_lists(current_user: unauth_user)).to be_nil
resolve_board_lists(current_user: unauth_user)
end.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end end
end end
...@@ -101,12 +99,6 @@ RSpec.describe Resolvers::BoardListsResolver do ...@@ -101,12 +99,6 @@ RSpec.describe Resolvers::BoardListsResolver do
end end
def resolve_board_lists(args: {}, current_user: user) def resolve_board_lists(args: {}, current_user: user)
context = GraphQL::Query::Context.new( resolve(described_class, obj: board, args: args, ctx: { current_user: current_user })
query: OpenStruct.new(schema: nil),
values: { current_user: current_user },
object: nil
)
resolve(described_class, obj: board, args: args, ctx: context )
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment