Commit 8d7a47f8 authored by charlie ablett's avatar charlie ablett Committed by Luke Duncalfe

Replace Authorize instrument with gem auth

This changes our GraphQL code to use the built-in `#authorize` methods
to handle permissions.

We originally implemented this functionality with a field-extension,
but this is no longer necessary. This commit replaces that unnecessary
field extension with implementations of `BaseObject#authorize` that
use our policy framework.

Significant changes included here:

- field authorization now works as per the library specification: it
  authorizes against the current object, not the resolved value.
  To apply permissions to the resolved value, use the type permissions.
- we allow resolvers to do the same (opt-in).
- we extend authorization to enums (currently no enums use
  authorization).

Note on enums:
We don't actually have any authorization on enums, but we need to detect
that efficiently. By supporting `ObjectAuthorization`, we can skip
redaction now, and support it later (if we add enum members that require
special authorization to see).

Removals:

- The ManualAuthorization temporary class
- The synchronized_object method on BaseResolver
- Field.authorize DSL method

Changes:

The error raised when there is no auth becomes an internal server
error (ConfigurationError) since it cannot be caused by the client,
and represents a programming mistake.

The board issue move mutation has unnecessary logic removed, and the
test for this is adjusted to verify the correctness of this change.
Co-authored-by: default avatarAlex Kalderimis <akalderimis@gitlab.com>
Co-authored-by: default avatarCharlie Ablett <cablett@gitlab.com>
parent 3f1c24ce
...@@ -35,7 +35,6 @@ class GraphqlController < ApplicationController ...@@ -35,7 +35,6 @@ class GraphqlController < ApplicationController
def execute def execute
result = multiplex? ? execute_multiplex : execute_query result = multiplex? ? execute_multiplex : execute_query
render json: result render json: result
end end
......
...@@ -12,7 +12,6 @@ class GitlabSchema < GraphQL::Schema ...@@ -12,7 +12,6 @@ class GitlabSchema < GraphQL::Schema
use GraphQL::Pagination::Connections use GraphQL::Pagination::Connections
use BatchLoader::GraphQL use BatchLoader::GraphQL
use Gitlab::Graphql::Authorize
use Gitlab::Graphql::Pagination::Connections use Gitlab::Graphql::Pagination::Connections
use Gitlab::Graphql::GenericTracing use Gitlab::Graphql::GenericTracing
use Gitlab::Graphql::Timeout, max_seconds: Gitlab.config.gitlab.graphql_timeout use Gitlab::Graphql::Timeout, max_seconds: Gitlab.config.gitlab.graphql_timeout
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Mutations module Mutations
class BaseMutation < GraphQL::Schema::RelayClassicMutation class BaseMutation < GraphQL::Schema::RelayClassicMutation
prepend Gitlab::Graphql::Authorize::AuthorizeResource include Gitlab::Graphql::Authorize::AuthorizeResource
prepend Gitlab::Graphql::CopyFieldDescription prepend Gitlab::Graphql::CopyFieldDescription
prepend ::Gitlab::Graphql::GlobalIDCompatibility prepend ::Gitlab::Graphql::GlobalIDCompatibility
...@@ -29,10 +29,30 @@ module Mutations ...@@ -29,10 +29,30 @@ module Mutations
def ready?(**args) def ready?(**args)
if Gitlab::Database.read_only? if Gitlab::Database.read_only?
raise Gitlab::Graphql::Errors::ResourceNotAvailable, ERROR_MESSAGE raise_resource_not_available_error! ERROR_MESSAGE
else else
true true
end end
end end
def load_application_object(argument, lookup_as_type, id, context)
::Gitlab::Graphql::Lazy.new { super }.catch(::GraphQL::UnauthorizedError) do |e|
Gitlab::ErrorTracking.track_exception(e)
# The default behaviour is to abort processing and return nil for the
# entire mutation field, but not set any top-level errors. We prefer to
# at least say that something went wrong.
raise_resource_not_available_error!
end
end
def self.authorized?(object, context)
# we never provide an object to mutations, but we do need to have a user.
context[:current_user].present? && !context[:current_user].blocked?
end
# See: AuthorizeResource#authorized_resource?
def self.authorization
@authorization ||= ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(authorize)
end
end end
end end
...@@ -52,13 +52,10 @@ module Mutations ...@@ -52,13 +52,10 @@ module Mutations
super super
end end
def resolve(board:, **args) def resolve(board:, project_path:, iid:, **args)
Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/247861') Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/247861')
raise_resource_not_available_error! unless board issue = authorized_find!(project_path: project_path, iid: iid)
authorize_board!(board)
issue = authorized_find!(project_path: args[:project_path], iid: args[:iid])
move_params = { id: issue.id, board_id: board.id }.merge(move_arguments(args)) move_params = { id: issue.id, board_id: board.id }.merge(move_arguments(args))
move_issue(board, issue, move_params) move_issue(board, issue, move_params)
...@@ -84,12 +81,6 @@ module Mutations ...@@ -84,12 +81,6 @@ module Mutations
def move_arguments(args) def move_arguments(args)
args.slice(:from_list_id, :to_list_id, :move_after_id, :move_before_id) args.slice(:from_list_id, :to_list_id, :move_after_id, :move_before_id)
end end
def authorize_board!(board)
return if Ability.allowed?(current_user, :read_issue_board, board.resource_parent)
raise_resource_not_available_error!
end
end end
end end
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Resolvers module Resolvers
module AlertManagement module AlertManagement
class HttpIntegrationsResolver < BaseResolver class HttpIntegrationsResolver < BaseResolver
alias_method :project, :synchronized_object alias_method :project, :object
type Types::AlertManagement::HttpIntegrationType.connection_type, null: true type Types::AlertManagement::HttpIntegrationType.connection_type, null: true
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Resolvers module Resolvers
module AlertManagement module AlertManagement
class IntegrationsResolver < BaseResolver class IntegrationsResolver < BaseResolver
alias_method :project, :synchronized_object alias_method :project, :object
type Types::AlertManagement::IntegrationType.connection_type, null: true type Types::AlertManagement::IntegrationType.connection_type, null: true
......
...@@ -138,16 +138,6 @@ module Resolvers ...@@ -138,16 +138,6 @@ module Resolvers
end end
end end
# TODO: remove! This should never be necessary
# Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/13984,
# since once we use that authorization approach, the object is guaranteed to
# be synchronized before any field.
def synchronized_object
strong_memoize(:synchronized_object) do
::Gitlab::Graphql::Lazy.force(object)
end
end
def single? def single?
false false
end end
...@@ -160,5 +150,13 @@ module Resolvers ...@@ -160,5 +150,13 @@ module Resolvers
def select_result(results) def select_result(results)
results results
end end
def self.authorization
@authorization ||= ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(try(:required_permissions))
end
def self.authorized?(object, context)
authorization.ok?(object, context[:current_user])
end
end end
end end
...@@ -3,13 +3,12 @@ ...@@ -3,13 +3,12 @@
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
include LooksAhead
type Types::BoardListType, null: true type Types::BoardListType, null: true
extras [:lookahead]
authorize :read_issue_board_list authorize :read_issue_board_list
authorizes_object!
argument :id, Types::GlobalIDType[List], argument :id, Types::GlobalIDType[List],
required: false, required: false,
...@@ -21,15 +20,11 @@ module Resolvers ...@@ -21,15 +20,11 @@ module Resolvers
alias_method :board, :object alias_method :board, :object
def resolve(lookahead: nil, id: nil, issue_filters: {}) def resolve_with_lookahead(id: nil, issue_filters: {})
authorize!(board)
lists = board_lists(id) lists = board_lists(id)
context.scoped_set!(:issue_filters, issue_filters(issue_filters)) context.scoped_set!(:issue_filters, issue_filters(issue_filters))
if load_preferences?(lookahead) List.preload_preferences_for_user(lists, current_user) if load_preferences?
List.preload_preferences_for_user(lists, current_user)
end
offset_pagination(lists) offset_pagination(lists)
end end
...@@ -46,9 +41,8 @@ module Resolvers ...@@ -46,9 +41,8 @@ module Resolvers
service.execute(board, create_default_lists: false) service.execute(board, create_default_lists: false)
end end
def load_preferences?(lookahead) def load_preferences?
lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed) || node_selection&.selects?(:collapsed)
lookahead&.selection(:nodes)&.selects?(:collapsed)
end end
def extract_list_id(gid) def extract_list_id(gid)
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Resolvers module Resolvers
class BoardResolver < BaseResolver.single class BoardResolver < BaseResolver.single
alias_method :parent, :synchronized_object alias_method :parent, :object
type Types::BoardType, null: true type Types::BoardType, null: true
......
# 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
...@@ -4,7 +4,7 @@ module Resolvers ...@@ -4,7 +4,7 @@ module Resolvers
class GroupMergeRequestsResolver < MergeRequestsResolver class GroupMergeRequestsResolver < MergeRequestsResolver
include GroupIssuableResolver include GroupIssuableResolver
alias_method :group, :synchronized_object alias_method :group, :object
type Types::MergeRequestType.connection_type, null: true type Types::MergeRequestType.connection_type, null: true
......
...@@ -4,7 +4,7 @@ module Resolvers ...@@ -4,7 +4,7 @@ module Resolvers
class MergeRequestResolver < BaseResolver.single class MergeRequestResolver < BaseResolver.single
include ResolvesMergeRequests include ResolvesMergeRequests
alias_method :project, :synchronized_object alias_method :project, :object
type ::Types::MergeRequestType, null: true type ::Types::MergeRequestType, null: true
......
...@@ -6,7 +6,7 @@ module Resolvers ...@@ -6,7 +6,7 @@ module Resolvers
type ::Types::MergeRequestType.connection_type, null: true type ::Types::MergeRequestType.connection_type, null: true
alias_method :project, :synchronized_object alias_method :project, :object
def self.accept_assignee def self.accept_assignee
argument :assignee_username, GraphQL::STRING_TYPE, argument :assignee_username, GraphQL::STRING_TYPE,
......
...@@ -56,7 +56,7 @@ module Resolvers ...@@ -56,7 +56,7 @@ module Resolvers
end end
def parent def parent
synchronized_object object
end end
def parent_id_parameters(args) def parent_id_parameters(args)
......
...@@ -3,11 +3,11 @@ ...@@ -3,11 +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 authorize :admin_project
authorizes_object!
argument :active, argument :active,
GraphQL::BOOLEAN_TYPE, GraphQL::BOOLEAN_TYPE,
...@@ -20,15 +20,7 @@ module Resolvers ...@@ -20,15 +20,7 @@ module Resolvers
alias_method :project, :object alias_method :project, :object
def resolve(**args) def resolve(active: nil, type: nil)
authorize!(project)
services(args[:active], args[:type])
end
private
def services(active, type)
servs = project.services servs = project.services
servs = servs.by_active_flag(active) unless active.nil? servs = servs.by_active_flag(active) unless active.nil?
servs = servs.by_type(type) unless type.blank? servs = servs.by_type(type) unless type.blank?
......
...@@ -3,12 +3,12 @@ ...@@ -3,12 +3,12 @@
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 authorize :read_snippet
calls_gitaly! calls_gitaly!
authorizes_object!
alias_method :snippet, :object alias_method :snippet, :object
...@@ -17,7 +17,6 @@ module Resolvers ...@@ -17,7 +17,6 @@ module Resolvers
description: 'Paths of the blobs.' description: 'Paths of the blobs.'
def resolve(paths: []) def resolve(paths: [])
authorize!(snippet)
return [snippet.blob] if snippet.empty_repo? return [snippet.blob] if snippet.empty_repo?
if paths.empty? if paths.empty?
......
...@@ -13,7 +13,7 @@ module Resolvers ...@@ -13,7 +13,7 @@ module Resolvers
description: 'The global ID of the project the authored merge requests should be in. Incompatible with projectPath.' description: 'The global ID of the project the authored merge requests should be in. Incompatible with projectPath.'
attr_reader :project attr_reader :project
alias_method :user, :synchronized_object alias_method :user, :object
def ready?(project_id: nil, project_path: nil, **args) def ready?(project_id: nil, project_path: nil, **args)
return early_return unless can_read_profile? return early_return unless can_read_profile?
......
...@@ -36,6 +36,18 @@ module Types ...@@ -36,6 +36,18 @@ module Types
def enum def enum
@enum_values ||= {}.with_indifferent_access @enum_values ||= {}.with_indifferent_access
end end
def authorization
@authorization ||= ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(authorize)
end
def authorize(*abilities)
@abilities = abilities
end
def authorized?(object, context)
authorization.ok?(object, context[:current_user])
end
end end
end end
end end
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
module Types module Types
class BaseField < GraphQL::Schema::Field class BaseField < GraphQL::Schema::Field
prepend Gitlab::Graphql::Authorize
include GitlabStyleDeprecations include GitlabStyleDeprecations
argument_class ::Types::BaseArgument argument_class ::Types::BaseArgument
...@@ -13,6 +12,7 @@ module Types ...@@ -13,6 +12,7 @@ module Types
@calls_gitaly = !!kwargs.delete(:calls_gitaly) @calls_gitaly = !!kwargs.delete(:calls_gitaly)
@constant_complexity = kwargs[:complexity].is_a?(Integer) && kwargs[:complexity] > 0 @constant_complexity = kwargs[:complexity].is_a?(Integer) && kwargs[:complexity] > 0
@requires_argument = !!kwargs.delete(:requires_argument) @requires_argument = !!kwargs.delete(:requires_argument)
@authorize = Array.wrap(kwargs.delete(:authorize))
kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity]) kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity])
@feature_flag = kwargs[:feature_flag] @feature_flag = kwargs[:feature_flag]
kwargs = check_feature_flag(kwargs) kwargs = check_feature_flag(kwargs)
...@@ -22,8 +22,8 @@ module Types ...@@ -22,8 +22,8 @@ module Types
# We want to avoid the overhead of this in prod # We want to avoid the overhead of this in prod
extension ::Gitlab::Graphql::CallsGitaly::FieldExtension if Gitlab.dev_or_test_env? extension ::Gitlab::Graphql::CallsGitaly::FieldExtension if Gitlab.dev_or_test_env?
extension ::Gitlab::Graphql::Present::FieldExtension extension ::Gitlab::Graphql::Present::FieldExtension
extension ::Gitlab::Graphql::Authorize::ConnectionFilterExtension
end end
def may_call_gitaly? def may_call_gitaly?
...@@ -34,6 +34,19 @@ module Types ...@@ -34,6 +34,19 @@ module Types
@requires_argument || arguments.values.any? { |argument| argument.type.non_null? } @requires_argument || arguments.values.any? { |argument| argument.type.non_null? }
end end
# By default fields authorize against the current object, but that is not how our
# resolvers work - they use declarative permissions to authorize fields
# manually (so we make them opt in).
# TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/300922
# (separate out authorize into permissions on the object, and on the
# resolved values)
# We do not support argument authorization in our schema. If/when we do,
# we should call `super` here, to apply argument authorization checks.
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/324647
def authorized?(object, args, ctx)
field_authorized?(object, ctx) && resolver_authorized?(object, ctx)
end
def base_complexity def base_complexity
complexity = DEFAULT_COMPLEXITY complexity = DEFAULT_COMPLEXITY
complexity += 1 if calls_gitaly? complexity += 1 if calls_gitaly?
...@@ -58,6 +71,26 @@ module Types ...@@ -58,6 +71,26 @@ module Types
attr_reader :feature_flag attr_reader :feature_flag
def field_authorized?(object, ctx)
authorization.ok?(object, ctx[:current_user])
end
# Historically our resolvers have used declarative permission checks only
# for _what they resolved_, not the _object they resolved these things from_
# We preserve these semantics here, and only apply resolver authorization
# if the resolver has opted in.
def resolver_authorized?(object, ctx)
if @resolver_class && @resolver_class.try(:authorizes_object?)
@resolver_class.authorized?(object, ctx)
else
true
end
end
def authorization
@authorization ||= ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(@authorize)
end
def feature_documentation_message(key, description) def feature_documentation_message(key, description)
"#{description} Available only when feature flag `#{key}` is enabled." "#{description} Available only when feature flag `#{key}` is enabled."
end end
......
...@@ -5,5 +5,11 @@ module Types ...@@ -5,5 +5,11 @@ module Types
include GraphQL::Schema::Interface include GraphQL::Schema::Interface
field_class ::Types::BaseField field_class ::Types::BaseField
definition_methods do
def authorized?(object, context)
resolve_type(object, context).authorized?(object, context)
end
end
end end
end end
...@@ -19,6 +19,14 @@ module Types ...@@ -19,6 +19,14 @@ module Types
GitlabSchema.id_from_object(object) GitlabSchema.id_from_object(object)
end end
def self.authorization
@authorization ||= ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(authorize)
end
def self.authorized?(object, context)
authorization.ok?(object, context[:current_user])
end
def current_user def current_user
context[:current_user] context[:current_user]
end end
......
...@@ -2,5 +2,8 @@ ...@@ -2,5 +2,8 @@
module Types module Types
class BaseUnion < GraphQL::Schema::Union class BaseUnion < GraphQL::Schema::Union
def self.authorized?(object, context)
resolve_type(object, context).authorized?(object, context)
end
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
GraphQL::ObjectType.accepts_definitions(authorize: GraphQL::Define.assign_metadata_key(:authorize)) GraphQL::ObjectType.accepts_definitions(authorize: GraphQL::Define.assign_metadata_key(:authorize))
GraphQL::Field.accepts_definitions(authorize: GraphQL::Define.assign_metadata_key(:authorize))
GraphQL::Schema::Object.accepts_definition(:authorize) GraphQL::Schema::Object.accepts_definition(:authorize)
GraphQL::Schema::Field.accepts_definition(:authorize)
...@@ -5,7 +5,7 @@ module Resolvers ...@@ -5,7 +5,7 @@ module Resolvers
class EpicsResolver < BaseResolver class EpicsResolver < BaseResolver
include ::BoardIssueFilterable include ::BoardIssueFilterable
alias_method :board, :synchronized_object alias_method :board, :object
argument :issue_filters, Types::Boards::BoardIssueInputType, argument :issue_filters, Types::Boards::BoardIssueInputType,
required: false, required: false,
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Resolvers module Resolvers
class DastSiteProfileResolver < BaseResolver class DastSiteProfileResolver < BaseResolver
alias_method :project, :synchronized_object alias_method :project, :object
type Types::DastSiteProfileType.connection_type, null: true type Types::DastSiteProfileType.connection_type, null: true
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Resolvers module Resolvers
class DastSiteValidationResolver < BaseResolver class DastSiteValidationResolver < BaseResolver
alias_method :project, :synchronized_object alias_method :project, :object
type Types::DastSiteValidationType.connection_type, null: true type Types::DastSiteValidationType.connection_type, null: true
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Resolvers module Resolvers
module IncidentManagement module IncidentManagement
class OncallScheduleResolver < BaseResolver class OncallScheduleResolver < BaseResolver
alias_method :project, :synchronized_object alias_method :project, :object
type Types::IncidentManagement::OncallScheduleType.connection_type, null: true type Types::IncidentManagement::OncallScheduleType.connection_type, null: true
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Resolvers module Resolvers
module IncidentManagement module IncidentManagement
class OncallShiftsResolver < BaseResolver class OncallShiftsResolver < BaseResolver
alias_method :rotation, :synchronized_object alias_method :rotation, :object
type Types::IncidentManagement::OncallShiftType.connection_type, null: true type Types::IncidentManagement::OncallShiftType.connection_type, null: true
......
...@@ -4,7 +4,7 @@ module Resolvers ...@@ -4,7 +4,7 @@ module Resolvers
class TimeboxReportResolver < BaseResolver class TimeboxReportResolver < BaseResolver
type Types::TimeboxReportType, null: true type Types::TimeboxReportType, null: true
alias_method :timebox, :synchronized_object alias_method :timebox, :object
def resolve(*args) def resolve(*args)
response = TimeboxReportService.new(timebox).execute response = TimeboxReportService.new(timebox).execute
......
...@@ -32,8 +32,7 @@ module Types ...@@ -32,8 +32,7 @@ module Types
description: 'Name of the vulnerability finding.' description: 'Name of the vulnerability finding.'
field :project, ::Types::ProjectType, null: true, field :project, ::Types::ProjectType, null: true,
description: 'The project on which the vulnerability finding was found.', description: 'The project on which the vulnerability finding was found.'
authorize: :read_project
field :description, GraphQL::STRING_TYPE, null: true, field :description, GraphQL::STRING_TYPE, null: true,
description: 'Description of the vulnerability finding.' description: 'Description of the vulnerability finding.'
......
...@@ -5,6 +5,10 @@ require 'spec_helper' ...@@ -5,6 +5,10 @@ require 'spec_helper'
RSpec.describe Resolvers::DastSiteProfileResolver do RSpec.describe Resolvers::DastSiteProfileResolver do
include GraphqlHelpers include GraphqlHelpers
before do
stub_licensed_features(security_on_demand_scans: true)
end
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:developer) { create(:user, developer_projects: [project] ) } let_it_be(:developer) { create(:user, developer_projects: [project] ) }
let_it_be(:dast_site_profile1) { create(:dast_site_profile, project: project) } let_it_be(:dast_site_profile1) { create(:dast_site_profile, project: project) }
...@@ -26,6 +30,20 @@ RSpec.describe Resolvers::DastSiteProfileResolver do ...@@ -26,6 +30,20 @@ RSpec.describe Resolvers::DastSiteProfileResolver do
subject { sync(dast_site_profiles) } subject { sync(dast_site_profiles) }
it { is_expected.to contain_exactly(dast_site_profile1, dast_site_profile2) } it { is_expected.to contain_exactly(dast_site_profile1, dast_site_profile2) }
context 'when the feature is disabled' do
before do
stub_licensed_features(security_on_demand_scans: false)
end
it { is_expected.to be_empty }
end
context 'when the user does not have access' do
let(:current_user) { create(:user) }
it { is_expected.to be_empty }
end
end end
private private
......
...@@ -206,14 +206,18 @@ RSpec.describe GitlabSchema.types['Project'] do ...@@ -206,14 +206,18 @@ RSpec.describe GitlabSchema.types['Project'] do
end end
describe 'compliance_frameworks' do describe 'compliance_frameworks' do
it 'queries in batches' do it 'queries in batches', :request_store, :use_clean_rails_memory_store_caching do
projects = create_list(:project, 2, :with_compliance_framework) projects = create_list(:project, 2, :with_compliance_framework)
projects.each { |p| p.add_maintainer(user) } projects.each do |p|
p.add_maintainer(user)
# Cache warm up: runs authorization for each user.
resolve_field(:id, p, current_user: user)
end
results = batch_sync(max_queries: 1) do results = batch_sync(max_queries: 1) do
projects.flat_map do |p| projects.flat_map do |p|
resolve_field(:compliance_frameworks, p) resolve_field(:compliance_frameworks, p, current_user: user)
end end
end end
frameworks = results.flat_map(&:to_a) frameworks = results.flat_map(&:to_a)
......
# frozen_string_literal: true
module Gitlab
module Graphql
# Allow fields to declare permissions their objects must have. The field
# will be set to nil unless all required permissions are present.
module Authorize
extend ActiveSupport::Concern
def self.use(schema_definition)
schema_definition.instrument(:field, Gitlab::Graphql::Authorize::Instrumentation.new, after_built_ins: true)
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Graphql
module Authorize
class AuthorizeFieldService
def initialize(field)
@field = field
@old_resolve_proc = @field.resolve_proc
end
def authorizations?
authorizations.present?
end
def authorized_resolve
proc do |parent_typed_object, args, ctx|
resolved_type = @old_resolve_proc.call(parent_typed_object, args, ctx)
authorizing_object = authorize_against(parent_typed_object, resolved_type)
filter_allowed(ctx[:current_user], resolved_type, authorizing_object)
end
end
private
def authorizations
@authorizations ||= (type_authorizations + field_authorizations).uniq
end
# Returns any authorize metadata from the return type of @field
def type_authorizations
type = @field.type
# When the return type of @field is a collection, find the singular type
if @field.connection?
type = node_type_for_relay_connection(type)
elsif type.list?
type = node_type_for_basic_connection(type)
end
type = type.unwrap if type.kind.non_null?
Array.wrap(type.metadata[:authorize])
end
# Returns any authorize metadata from @field
def field_authorizations
return [] if @field.metadata[:authorize] == true
Array.wrap(@field.metadata[:authorize])
end
def authorize_against(parent_typed_object, resolved_type)
if scalar_type?
# The field is a built-in/scalar type, or a list of scalars
# authorize using the parent's object
parent_typed_object.object
elsif @field.connection? || @field.type.list? || resolved_type.is_a?(Array)
# The field is a connection or a list of non-built-in types, we'll
# authorize each element when rendering
nil
elsif resolved_type.respond_to?(:object)
# The field is a type representing a single object, we'll authorize
# against the object directly
resolved_type.object
else
# Resolved type is a single object that might not be loaded yet by
# the batchloader, we'll authorize that
resolved_type
end
end
def filter_allowed(current_user, resolved_type, authorizing_object)
if resolved_type.nil?
# We're not rendering anything, for example when a record was not found
# no need to do anything
elsif authorizing_object
# Authorizing fields representing scalars, or a simple field with an object
::Gitlab::Graphql::Lazy.with_value(authorizing_object) do |object|
resolved_type if allowed_access?(current_user, object)
end
elsif @field.connection?
::Gitlab::Graphql::Lazy.with_value(resolved_type) do |type|
# A connection with pagination, modify the visible nodes on the
# connection type in place
nodes = to_nodes(type)
nodes.keep_if { |node| allowed_access?(current_user, node) } if nodes
type
end
elsif @field.type.list? || resolved_type.is_a?(Array)
# A simple list of rendered types each object being an object to authorize
::Gitlab::Graphql::Lazy.with_value(resolved_type) do |items|
items.select do |single_object_type|
object_type = realized(single_object_type)
object = object_type.try(:object) || object_type
allowed_access?(current_user, object)
end
end
else
raise "Can't authorize #{@field}"
end
end
# Ensure that we are dealing with realized objects, not delayed promises
def realized(thing)
::Gitlab::Graphql::Lazy.force(thing)
end
# Try to get the connection
# can be at type.object or at type
def to_nodes(type)
if type.respond_to?(:nodes)
type.nodes
elsif type.respond_to?(:object)
to_nodes(type.object)
else
nil
end
end
def allowed_access?(current_user, object)
object = realized(object)
authorizations.all? do |ability|
Ability.allowed?(current_user, ability, object)
end
end
# Returns the singular type for relay connections.
# This will be the type class of edges.node
def node_type_for_relay_connection(type)
type.unwrap.get_field('edges').type.unwrap.get_field('node').type
end
# Returns the singular type for basic connections, for example `[Types::ProjectType]`
def node_type_for_basic_connection(type)
type.unwrap
end
def scalar_type?
node_type_for_basic_connection(@field.type).kind.scalar?
end
end
end
end
end
...@@ -5,15 +5,17 @@ module Gitlab ...@@ -5,15 +5,17 @@ module Gitlab
module Authorize module Authorize
module AuthorizeResource module AuthorizeResource
extend ActiveSupport::Concern extend ActiveSupport::Concern
ConfigurationError = Class.new(StandardError)
RESOURCE_ACCESS_ERROR = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" RESOURCE_ACCESS_ERROR = "The resource that you are attempting to access does " \
"not exist or you don't have permission to perform this action"
class_methods do class_methods do
def required_permissions def required_permissions
# If the `#authorize` call is used on multiple classes, we add the # If the `#authorize` call is used on multiple classes, we add the
# permissions specified on a subclass, to the ones that were specified # permissions specified on a subclass, to the ones that were specified
# on it's superclass. # on its superclass.
@required_permissions ||= if self.respond_to?(:superclass) && superclass.respond_to?(:required_permissions) @required_permissions ||= if respond_to?(:superclass) && superclass.respond_to?(:required_permissions)
superclass.required_permissions.dup superclass.required_permissions.dup
else else
[] []
...@@ -23,6 +25,18 @@ module Gitlab ...@@ -23,6 +25,18 @@ module Gitlab
def authorize(*permissions) def authorize(*permissions)
required_permissions.concat(permissions) required_permissions.concat(permissions)
end end
def authorizes_object?
defined?(@authorizes_object) ? @authorizes_object : false
end
def authorizes_object!
@authorizes_object = true
end
def raise_resource_not_available_error!(msg = RESOURCE_ACCESS_ERROR)
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, msg
end
end end
def find_object(*args) def find_object(*args)
...@@ -37,33 +51,21 @@ module Gitlab ...@@ -37,33 +51,21 @@ module Gitlab
object object
end end
# authorizes the object using the current class authorization.
def authorize!(object) def authorize!(object)
unless authorized_resource?(object) raise_resource_not_available_error! unless authorized_resource?(object)
raise_resource_not_available_error!
end
end end
# this was named `#authorized?`, however it conflicts with the native
# graphql gem version
# TODO consider adopting the gem's built in authorization system
# https://gitlab.com/gitlab-org/gitlab/issues/13984
def authorized_resource?(object) def authorized_resource?(object)
# Sanity check. We don't want to accidentally allow a developer to authorize # Sanity check. We don't want to accidentally allow a developer to authorize
# without first adding permissions to authorize against # without first adding permissions to authorize against
if self.class.required_permissions.empty? raise ConfigurationError, "#{self.class.name} has no authorizations" if self.class.authorization.none?
raise Gitlab::Graphql::Errors::ArgumentError, "#{self.class.name} has no authorizations"
end
self.class.required_permissions.all? do |ability| self.class.authorization.ok?(object, current_user)
# The actions could be performed across multiple objects. In which
# case the current user is common, and we could benefit from the
# caching in `DeclarativePolicy`.
Ability.allowed?(current_user, ability, object, scope: :user)
end
end end
def raise_resource_not_available_error!(msg = RESOURCE_ACCESS_ERROR) def raise_resource_not_available_error!(*args)
raise Gitlab::Graphql::Errors::ResourceNotAvailable, msg self.class.raise_resource_not_available_error!(*args)
end end
end end
end end
......
# frozen_string_literal: true
module Gitlab
module Graphql
module Authorize
class ConnectionFilterExtension < GraphQL::Schema::FieldExtension
class Redactor
include ::Gitlab::Graphql::Laziness
def initialize(type, context)
@type = type
@context = context
end
def redact(nodes)
remove_unauthorized(nodes)
nodes
end
def active?
# some scalar types (such as integers) do not respond to :authorized?
return false unless @type.respond_to?(:authorized?)
auth = @type.try(:authorization)
auth.nil? || auth.any?
end
private
def remove_unauthorized(nodes)
nodes
.map! { |lazy| force(lazy) }
.keep_if { |forced| @type.authorized?(forced, @context) }
end
end
def after_resolve(value:, context:, **rest)
if @field.connection?
redact_connection(value, context)
elsif @field.type.list?
redact_list(value.to_a, context) unless value.nil?
end
value
end
def redact_connection(conn, context)
redactor = Redactor.new(@field.type.unwrap.node_type, context)
return unless redactor.active?
conn.redactor = redactor if conn.respond_to?(:redactor=)
end
def redact_list(list, context)
redactor = Redactor.new(@field.type.unwrap, context)
redactor.redact(list) if redactor.active?
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Graphql
module Authorize
class Instrumentation
# Replace the resolver for the field with one that will only return the
# resolved object if the permissions check is successful.
def instrument(_type, field)
service = AuthorizeFieldService.new(field)
if service.authorizations?
field.redefine { resolve(service.authorized_resolve) }
else
field
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Graphql
module Authorize
class ObjectAuthorization
attr_reader :abilities
def initialize(abilities)
@abilities = Array.wrap(abilities).flatten
end
def none?
abilities.empty?
end
def any?
abilities.present?
end
def ok?(object, current_user)
return true if none?
abilities.all? do |ability|
Ability.allowed?(current_user, ability, object)
end
end
end
end
end
end
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Gitlab::Graphql::Authorize' do RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do
include GraphqlHelpers include GraphqlHelpers
include Graphql::ResolverFactories include Graphql::ResolverFactories
...@@ -10,10 +10,14 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -10,10 +10,14 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
let(:permission_single) { :foo } let(:permission_single) { :foo }
let(:permission_collection) { [:foo, :bar] } let(:permission_collection) { [:foo, :bar] }
let(:test_object) { double(name: 'My name') } let(:test_object) { double(name: 'My name') }
let(:authorizing_object) { test_object }
# to override when combining permissions
let(:permission_object_one) { authorizing_object }
let(:permission_object_two) { authorizing_object }
let(:query_string) { '{ item { name } }' } let(:query_string) { '{ item { name } }' }
let(:result) do let(:result) do
schema = empty_schema schema = empty_schema
schema.use(Gitlab::Graphql::Authorize)
execute_query(query_type, schema: schema) execute_query(query_type, schema: schema)
end end
...@@ -33,18 +37,25 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -33,18 +37,25 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
shared_examples 'authorization with a collection of permissions' do shared_examples 'authorization with a collection of permissions' do
it 'returns the protected field when user has all permissions' do it 'returns the protected field when user has all permissions' do
permit(*permission_collection) permit_on(permission_object_one, permission_collection.first)
permit_on(permission_object_two, permission_collection.second)
expect(subject).to eq('name' => test_object.name) expect(subject).to eq('name' => test_object.name)
end end
it 'returns nil when user only has one of the permissions' do it 'returns nil when user only has one of the permissions' do
permit(permission_collection.first) permit_on(permission_object_one, permission_collection.first)
expect(subject).to be_nil
end
it 'returns nil when user only has the other of the permissions' do
permit_on(permission_object_two, permission_collection.second)
expect(subject).to be_nil expect(subject).to be_nil
end end
it 'returns nil when user only has none of the permissions' do it 'returns nil when user has neither of the required permissions' do
expect(subject).to be_nil expect(subject).to be_nil
end end
end end
...@@ -56,6 +67,7 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -56,6 +67,7 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
describe 'Field authorizations' do describe 'Field authorizations' do
let(:type) { type_factory } let(:type) { type_factory }
let(:authorizing_object) { nil }
describe 'with a single permission' do describe 'with a single permission' do
let(:query_type) do let(:query_type) do
...@@ -71,9 +83,10 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -71,9 +83,10 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
let(:query_type) do let(:query_type) do
permissions = permission_collection permissions = permission_collection
query_factory do |qt| query_factory do |qt|
qt.field :item, type, null: true, resolver: new_resolver(test_object) do qt.field :item, type,
authorize permissions null: true,
end resolver: new_resolver(test_object),
authorize: permissions
end end
end end
...@@ -110,9 +123,8 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -110,9 +123,8 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
let(:type) do let(:type) do
permissions = permission_collection permissions = permission_collection
type_factory do |type| type_factory do |type|
type.field :name, GraphQL::STRING_TYPE, null: true do type.field :name, GraphQL::STRING_TYPE, null: true,
authorize permissions authorize: permissions
end
end end
end end
...@@ -163,6 +175,7 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -163,6 +175,7 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
end end
describe 'type and field authorizations together' do describe 'type and field authorizations together' do
let(:authorizing_object) { anything }
let(:permission_1) { permission_collection.first } let(:permission_1) { permission_collection.first }
let(:permission_2) { permission_collection.last } let(:permission_2) { permission_collection.last }
...@@ -181,7 +194,62 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -181,7 +194,62 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
include_examples 'authorization with a collection of permissions' include_examples 'authorization with a collection of permissions'
end end
describe 'type authorizations when applied to a relay connection' do describe 'resolver and field authorizations together' do
let(:permission_1) { permission_collection.first }
let(:permission_2) { permission_collection.last }
let(:type) { type_factory }
let(:query_type) do
query_factory do |query|
query.field :item, type, null: true,
resolver: resolver,
authorize: permission_2
end
end
context 'when the resolver authorizes the object' do
let(:permission_object_one) { be_nil }
let(:permission_object_two) { be_nil }
let(:resolver) do
resolver = simple_resolver(test_object)
resolver.include(::Gitlab::Graphql::Authorize::AuthorizeResource)
resolver.authorize permission_1
resolver.authorizes_object!
resolver
end
include_examples 'authorization with a collection of permissions'
end
context 'when the resolver does not authorize the object, but instead calls authorized_find!' do
let(:permission_object_one) { test_object }
let(:permission_object_two) { be_nil }
let(:resolver) do
resolver = new_resolver(test_object, method: :find_object)
resolver.authorize permission_1
resolver
end
include_examples 'authorization with a collection of permissions'
end
context 'when the resolver calls authorized_find!, but does not list any permissions' do
let(:permission_object_two) { be_nil }
let(:resolver) do
resolver = new_resolver(test_object, method: :find_object)
resolver
end
it 'raises a configuration error' do
permit_on(permission_object_two, permission_collection.second)
expect { execute_query(query_type) }
.to raise_error(::Gitlab::Graphql::Authorize::AuthorizeResource::ConfigurationError)
end
end
end
describe 'when type authorizations when applied to a relay connection' do
let(:query_string) { '{ item { edges { node { name } } } }' } let(:query_string) { '{ item { edges { node { name } } } }' }
let(:second_test_object) { double(name: 'Second thing') } let(:second_test_object) { double(name: 'Second thing') }
...@@ -303,8 +371,12 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do ...@@ -303,8 +371,12 @@ RSpec.describe 'Gitlab::Graphql::Authorize' do
private private
def permit(*permissions) def permit(*permissions)
permit_on(authorizing_object, *permissions)
end
def permit_on(object, *permissions)
permissions.each do |permission| permissions.each do |permission|
allow(Ability).to receive(:allowed?).with(user, permission, test_object).and_return(true) allow(Ability).to receive(:allowed?).with(user, permission, object).and_return(true)
end end
end end
end end
...@@ -14,10 +14,6 @@ RSpec.describe GitlabSchema do ...@@ -14,10 +14,6 @@ RSpec.describe GitlabSchema do
expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::GenericTracing)) expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::GenericTracing))
end end
it 'enables the authorization instrumenter' do
expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize::Instrumentation))
end
it 'has the base mutation' do it 'has the base mutation' do
expect(described_class.mutation).to eq(::Types::MutationType) expect(described_class.mutation).to eq(::Types::MutationType)
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::Boards::Issues::IssueMoveList do RSpec.describe Mutations::Boards::Issues::IssueMoveList do
include GraphqlHelpers
let_it_be(:group) { create(:group, :public) } let_it_be(:group) { create(:group, :public) }
let_it_be(:project) { create(:project, group: group) } let_it_be(:project) { create(:project, group: group) }
let_it_be(:board) { create(:board, group: group) } let_it_be(:board) { create(:board, group: group) }
...@@ -16,9 +18,8 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do ...@@ -16,9 +18,8 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do
let_it_be(:existing_issue1) { create(:labeled_issue, project: project, labels: [testing], relative_position: 10) } let_it_be(:existing_issue1) { create(:labeled_issue, project: project, labels: [testing], relative_position: 10) }
let_it_be(:existing_issue2) { create(:labeled_issue, project: project, labels: [testing], relative_position: 50) } let_it_be(:existing_issue2) { create(:labeled_issue, project: project, labels: [testing], relative_position: 50) }
let(:current_user) { user } let(:current_ctx) { { current_user: user } }
let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } let(:params) { { board_id: global_id_of(board), project_path: project.full_path, iid: issue1.iid } }
let(:params) { { board: board, project_path: project.full_path, iid: issue1.iid } }
let(:move_params) do let(:move_params) do
{ {
from_list_id: list1.id, from_list_id: list1.id,
...@@ -33,26 +34,45 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do ...@@ -33,26 +34,45 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do
group.add_guest(guest) group.add_guest(guest)
end end
subject do describe '#resolve' do
mutation.resolve(**params.merge(move_params)) subject do
end sync(resolve(described_class, args: params.merge(move_params), ctx: current_ctx))
end
%i[from_list_id to_list_id].each do |arg_name|
context "when we only pass #{arg_name}" do
let(:move_params) { { arg_name => list1.id } }
describe '#ready?' do it 'raises an error' do
it 'raises an error if required arguments are missing' do expect { subject }.to raise_error(
expect { mutation.ready?(**params) } Gitlab::Graphql::Errors::ArgumentError,
.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "At least one of the arguments " \ 'Both fromListId and toListId must be present'
"fromListId, toListId, afterId or beforeId is required") )
end
end
end end
it 'raises an error if only one of fromListId and toListId is present' do context 'when required arguments are missing' do
expect { mutation.ready?(**params.merge(from_list_id: list1.id)) } let(:move_params) { {} }
.to raise_error(Gitlab::Graphql::Errors::ArgumentError,
'Both fromListId and toListId must be present' it 'raises an error' do
expect { subject }.to raise_error(
Gitlab::Graphql::Errors::ArgumentError,
"At least one of the arguments fromListId, toListId, afterId or beforeId is required"
) )
end
end
context 'when the board ID is wrong' do
before do
params[:board_id] = global_id_of(project)
end
it 'raises an error' do
expect { subject }.to raise_error(::GraphQL::LoadApplicationObjectFailedError)
end
end end
end
describe '#resolve' do
context 'when user have access to resources' do context 'when user have access to resources' do
it 'moves and repositions issue' do it 'moves and repositions issue' do
subject subject
...@@ -63,15 +83,11 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do ...@@ -63,15 +83,11 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do
end end
end end
context 'when user have no access to resources' do context 'when user cannot update issue' do
shared_examples 'raises a resource not available error' do let(:current_ctx) { { current_user: guest } }
it { expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) }
end
context 'when user cannot update issue' do
let(:current_user) { guest }
it_behaves_like 'raises a resource not available error' specify do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end end
end end
end end
......
...@@ -32,6 +32,10 @@ RSpec.describe Mutations::DesignManagement::Upload do ...@@ -32,6 +32,10 @@ RSpec.describe Mutations::DesignManagement::Upload do
end end
context "when the feature is not available" do context "when the feature is not available" do
before do
enable_design_management(false)
end
it_behaves_like "resource not available" it_behaves_like "resource not available"
end end
...@@ -99,20 +103,20 @@ RSpec.describe Mutations::DesignManagement::Upload do ...@@ -99,20 +103,20 @@ RSpec.describe Mutations::DesignManagement::Upload do
it_behaves_like "resource not available" it_behaves_like "resource not available"
end end
context "a valid design" do context "with a valid design" do
it "returns the updated designs" do it "returns the updated designs" do
expect(resolve[:errors]).to eq [] expect(resolve[:errors]).to eq []
expect(resolve[:designs].map(&:filename)).to contain_exactly("dk.png") expect(resolve[:designs].map(&:filename)).to contain_exactly("dk.png")
end end
end end
context "context when passing an invalid project" do context "when passing an invalid project" do
let(:project) { build(:project) } let(:project) { build(:project) }
it_behaves_like "resource not available" it_behaves_like "resource not available"
end end
context "context when passing an invalid issue" do context "when passing an invalid issue" do
let(:issue) { build(:issue) } let(:issue) { build(:issue) }
it_behaves_like "resource not available" it_behaves_like "resource not available"
......
...@@ -38,11 +38,8 @@ RSpec.describe LooksAhead do ...@@ -38,11 +38,8 @@ RSpec.describe LooksAhead do
user = Class.new(GraphQL::Schema::Object) do user = Class.new(GraphQL::Schema::Object) do
graphql_name 'User' graphql_name 'User'
field :name, String, null: true field :name, String, null: true
field :issues, issue.connection_type, field :issues, issue.connection_type, null: true
null: true field :issues_with_lookahead, issue.connection_type, resolver: issues_resolver, null: true
field :issues_with_lookahead, issue.connection_type,
resolver: issues_resolver,
null: true
end end
Class.new(GraphQL::Schema) do Class.new(GraphQL::Schema) do
...@@ -101,7 +98,7 @@ RSpec.describe LooksAhead do ...@@ -101,7 +98,7 @@ RSpec.describe LooksAhead do
expect(res['errors']).to be_blank expect(res['errors']).to be_blank
expect(res.dig('data', 'findUser', 'name')).to eq(the_user.name) expect(res.dig('data', 'findUser', 'name')).to eq(the_user.name)
%w(issues issuesWithLookahead).each do |field| %w[issues issuesWithLookahead].each do |field|
expect(all_issue_titles(res, field)).to match_array(issue_titles) expect(all_issue_titles(res, field)).to match_array(issue_titles)
expect(all_label_ids(res, field)).to match_array(expected_label_ids) expect(all_label_ids(res, field)).to match_array(expected_label_ids)
end end
......
...@@ -41,7 +41,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do ...@@ -41,7 +41,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do
# AND "merge_requests"."iid" = 1 ORDER BY "merge_requests"."id" DESC # AND "merge_requests"."iid" = 1 ORDER BY "merge_requests"."id" DESC
# SELECT "projects".* FROM "projects" WHERE "projects"."id" = 2 # SELECT "projects".* FROM "projects" WHERE "projects"."id" = 2
# SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 2 # SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 2
let(:queries_per_project) { 3 } let(:queries_per_project) { 4 }
context 'no arguments' do context 'no arguments' do
it 'returns all merge requests' do it 'returns all merge requests' do
......
...@@ -48,15 +48,21 @@ RSpec.describe GitlabSchema.types['AlertManagementPrometheusIntegration'] do ...@@ -48,15 +48,21 @@ RSpec.describe GitlabSchema.types['AlertManagementPrometheusIntegration'] do
end end
end end
context 'without project' do context 'group integration' do
let_it_be(:integration) { create(:prometheus_service, project: nil, group: create(:group)) } let_it_be(:group) { create(:group) }
let_it_be(:integration) { create(:prometheus_service, project: nil, group: group) }
it_behaves_like 'has field with value', 'token' do
let(:value) { nil } # Since it is impossible to authorize the parent here, given that the
end # project is nil, all fields should be redacted:
it_behaves_like 'has field with value', 'url' do described_class.fields.keys.each do |field_name|
let(:value) { nil } context "field: #{field_name}" do
it 'is redacted' do
expect do
resolve_field(field_name, integration, current_user: user)
end.to raise_error(GraphqlHelpers::UnauthorizedObject)
end
end
end end
end end
end end
......
This diff is collapsed.
# frozen_string_literal: true
require 'spec_helper'
# Also see spec/graphql/features/authorization_spec.rb for
# integration tests of AuthorizeFieldService
RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
def type(type_authorizations = [])
Class.new(Types::BaseObject) do
graphql_name 'TestType'
authorize type_authorizations
end
end
def type_with_field(field_type, field_authorizations = [], resolved_value = 'Resolved value', **options)
Class.new(Types::BaseObject) do
graphql_name 'TestTypeWithField'
options.reverse_merge!(null: true)
field :test_field, field_type,
authorize: field_authorizations,
**options
define_method :test_field do
resolved_value
end
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) { 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) }
let(:type_class) { type_with_field(custom_type, :read_field, presented_object) }
let(:type_instance) { type_class.authorized_new(presented_object, context) }
let(:field) { type_class.fields['testField'].to_graphql }
subject(:resolved) { ::Gitlab::Graphql::Lazy.force(resolve) }
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) }
let(:upcaser) do
Module.new do
def self.upcase(strs)
strs.map(&:upcase)
end
end
end
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
it 'does not run authorizations until we force the resolved value' do
expect(Ability).not_to receive(:allowed?)
expect(resolve).to respond_to(:force)
end
it 'runs authorizations when we force the resolved value' do
spy_ability_check_for(ability, 'A')
expect(resolved).to eq('Resolved value')
end
it 'redacts values that fail the permissions check' do
spy_ability_check_for(ability, 'A', passed: false)
expect(resolved).to be_nil
end
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 '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)
a = resolve('a')
b = resolve('b')
c = resolve('c')
expect(a.force).to be_present
expect(b.force).to be_nil
expect(c.force).to be_present
end
end
end
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
expect(resolved).to eq('Resolved value')
end
it 'returns nil if the value was not authorized' do
allow(Ability).to receive(:allowed?).and_return false
expect(resolved).to be_nil
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_behaves_like 'checking permissions on the presented object'
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_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 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
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) }
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 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)
expect(resolved).to be_nil
end
context 'when the field is not nullable' do
let(:type_class) { type_with_field(custom_type, :read_field, object_in_field, null: false) }
it 'returns nil when viewing is not allowed' do
spy_ability_check_for(:read_type, object_in_field, passed: false)
expect(resolved).to be_nil
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)] }
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_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).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
def spy_ability_check_for(ability, object, passed: true)
expect(Ability)
.to receive(:allowed?)
.with(current_user, ability, object)
.and_return(passed)
end
end
...@@ -22,6 +22,14 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do ...@@ -22,6 +22,14 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do
def current_user def current_user
user user
end end
def context
{ current_user: user }
end
def self.authorization
@authorization ||= ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(required_permissions)
end
end end
end end
...@@ -30,9 +38,16 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do ...@@ -30,9 +38,16 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do
subject(:loading_resource) { fake_class.new(user, project) } subject(:loading_resource) { fake_class.new(user, project) }
before do
# don't allow anything by default
allow(Ability).to receive(:allowed?) do
false
end
end
context 'when the user is allowed to perform the action' do context 'when the user is allowed to perform the action' do
before do before do
allow(Ability).to receive(:allowed?).with(user, :read_the_thing, project, scope: :user) do allow(Ability).to receive(:allowed?).with(user, :read_the_thing, project) do
true true
end end
end end
...@@ -48,24 +63,12 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do ...@@ -48,24 +63,12 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do
expect { loading_resource.authorize!(project) }.not_to raise_error expect { loading_resource.authorize!(project) }.not_to raise_error
end end
end end
describe '#authorized_resource?' do
it 'is true' do
expect(loading_resource.authorized_resource?(project)).to be(true)
end
end
end end
context 'when the user is not allowed to perform the action' do context 'when the user is not allowed to perform the action' do
before do
allow(Ability).to receive(:allowed?).with(user, :read_the_thing, project, scope: :user) do
false
end
end
describe '#authorized_find!' do describe '#authorized_find!' do
it 'raises an error' do it 'raises an error' do
expect { loading_resource.authorize!(project) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) expect { loading_resource.authorized_find! }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end end
end end
...@@ -74,12 +77,6 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do ...@@ -74,12 +77,6 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do
expect { loading_resource.authorize!(project) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) expect { loading_resource.authorize!(project) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end end
end end
describe '#authorized_resource?' do
it 'is false' do
expect(loading_resource.authorized_resource?(project)).to be(false)
end
end
end end
context 'when the class does not define #find_object' do context 'when the class does not define #find_object' do
...@@ -92,46 +89,6 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do ...@@ -92,46 +89,6 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do
end end
end end
context 'when the class does not define authorize' do
let(:fake_class) do
Class.new do
include Gitlab::Graphql::Authorize::AuthorizeResource
attr_reader :user, :found_object
def initialize(user, found_object)
@user, @found_object = user, found_object
end
def find_object(*_args)
found_object
end
def current_user
user
end
def self.name
'TestClass'
end
end
end
let(:error) { /#{fake_class.name} has no authorizations/ }
describe '#authorized_find!' do
it 'raises a comprehensive error message' do
expect { loading_resource.authorized_find! }.to raise_error(error)
end
end
describe '#authorized_resource?' do
it 'raises a comprehensive error message' do
expect { loading_resource.authorized_resource?(project) }.to raise_error(error)
end
end
end
describe '#authorize' do describe '#authorize' do
it 'adds permissions from subclasses to those of superclasses when used on classes' do it 'adds permissions from subclasses to those of superclasses when used on classes' do
base_class = Class.new do base_class = Class.new do
......
...@@ -21,7 +21,8 @@ RSpec.describe 'Reposition and move issue within board lists' do ...@@ -21,7 +21,8 @@ RSpec.describe 'Reposition and move issue within board lists' do
let(:mutation_name) { mutation_class.graphql_name } let(:mutation_name) { mutation_class.graphql_name }
let(:mutation_result_identifier) { mutation_name.camelize(:lower) } let(:mutation_result_identifier) { mutation_name.camelize(:lower) }
let(:current_user) { user } let(:current_user) { user }
let(:params) { { board_id: board.to_global_id.to_s, project_path: project.full_path, iid: issue1.iid.to_s } } let(:board_id) { global_id_of(board) }
let(:params) { { board_id: board_id, project_path: project.full_path, iid: issue1.iid.to_s } }
let(:issue_move_params) do let(:issue_move_params) do
{ {
from_list_id: list1.id, from_list_id: list1.id,
...@@ -34,16 +35,44 @@ RSpec.describe 'Reposition and move issue within board lists' do ...@@ -34,16 +35,44 @@ RSpec.describe 'Reposition and move issue within board lists' do
end end
shared_examples 'returns an error' do shared_examples 'returns an error' do
it 'fails with error' do let(:message) do
message = "The resource that you are attempting to access does not exist or you don't have "\ "The resource that you are attempting to access does not exist or you don't have " \
"permission to perform this action" "permission to perform this action"
end
it 'fails with error' do
post_graphql_mutation(mutation(params), current_user: current_user) post_graphql_mutation(mutation(params), current_user: current_user)
expect(graphql_errors).to include(a_hash_including('message' => message)) expect(graphql_errors).to include(a_hash_including('message' => message))
end end
end end
context 'when the board_id is not a board' do
let(:board_id) { global_id_of(project) }
let(:issue_move_params) do
{ move_after_id: existing_issue1.id, move_before_id: existing_issue2.id }
end
it_behaves_like 'returns an error' do
let(:message) { include('does not represent an instance of') }
end
end
# This test aims to distinguish between the failures to authorize
# :read_issue_board and :update_issue
context 'when the user cannot read the issue board' do
let(:issue_move_params) do
{ move_after_id: existing_issue1.id, move_before_id: existing_issue2.id }
end
before do
allow(Ability).to receive(:allowed?).with(any_args).and_return(true)
allow(Ability).to receive(:allowed?).with(current_user, :read_issue_board, board).and_return(false)
end
it_behaves_like 'returns an error'
end
context 'when user has access to resources' do context 'when user has access to resources' do
context 'when repositioning an issue' do context 'when repositioning an issue' do
let(:issue_move_params) { { move_after_id: existing_issue1.id, move_before_id: existing_issue2.id } } let(:issue_move_params) { { move_after_id: existing_issue1.id, move_before_id: existing_issue2.id } }
......
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