From 703d0246ff6647802c0e2ddb064d0360b8fcfb94 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Tue, 18 Jun 2019 18:31:06 +0200 Subject: [PATCH] Add authorize to LabelType and NamespaceType This also disables the cop with a reasoning in types where appropriate --- app/graphql/types/ci/detailed_status_type.rb | 3 +++ app/graphql/types/issue_state_enum.rb | 3 +++ app/graphql/types/label_type.rb | 2 ++ app/graphql/types/merge_request_state_enum.rb | 3 +++ app/graphql/types/metadata_type.rb | 2 ++ app/graphql/types/namespace_type.rb | 2 ++ app/graphql/types/notes/diff_position_type.rb | 3 +++ app/graphql/types/project_type.rb | 2 +- app/graphql/types/query_type.rb | 5 +---- app/graphql/types/task_completion_status.rb | 4 ++++ app/graphql/types/tree/blob_type.rb | 3 +++ app/graphql/types/tree/submodule_type.rb | 3 +++ app/graphql/types/tree/tree_entry_type.rb | 3 +++ app/graphql/types/tree/tree_type.rb | 3 +++ spec/graphql/types/label_type_spec.rb | 2 ++ spec/graphql/types/metadata_type_spec.rb | 1 + spec/graphql/types/namespace_type_spec.rb | 2 ++ spec/graphql/types/query_type_spec.rb | 4 ---- spec/requests/api/graphql/namespace/projects_spec.rb | 4 +--- 19 files changed, 42 insertions(+), 12 deletions(-) diff --git a/app/graphql/types/ci/detailed_status_type.rb b/app/graphql/types/ci/detailed_status_type.rb index 2987354b556..5f7d7a934ce 100644 --- a/app/graphql/types/ci/detailed_status_type.rb +++ b/app/graphql/types/ci/detailed_status_type.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Types module Ci + # rubocop: disable Graphql/AuthorizeTypes + # This is presented through `PipelineType` that has its own authorization class DetailedStatusType < BaseObject graphql_name 'DetailedStatus' @@ -13,5 +15,6 @@ module Types field :text, GraphQL::STRING_TYPE, null: false field :tooltip, GraphQL::STRING_TYPE, null: false, method: :status_tooltip end + # rubocop: enable Graphql/AuthorizeTypes end end diff --git a/app/graphql/types/issue_state_enum.rb b/app/graphql/types/issue_state_enum.rb index 6521407fc9d..70c34fbe491 100644 --- a/app/graphql/types/issue_state_enum.rb +++ b/app/graphql/types/issue_state_enum.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true module Types + # rubocop: disable Graphql/AuthorizeTypes + # This is a BaseEnum through IssuableEnum, so it does not need authorization class IssueStateEnum < IssuableStateEnum graphql_name 'IssueState' description 'State of a GitLab issue' end + # rubocop: enable Graphql/AuthorizeTypes end diff --git a/app/graphql/types/label_type.rb b/app/graphql/types/label_type.rb index 50eb1b89c61..3aeda2e7953 100644 --- a/app/graphql/types/label_type.rb +++ b/app/graphql/types/label_type.rb @@ -4,6 +4,8 @@ module Types class LabelType < BaseObject graphql_name 'Label' + authorize :read_label + field :description, GraphQL::STRING_TYPE, null: true markdown_field :description_html, null: true field :title, GraphQL::STRING_TYPE, null: false diff --git a/app/graphql/types/merge_request_state_enum.rb b/app/graphql/types/merge_request_state_enum.rb index 92f52726ab3..37c890a3c8d 100644 --- a/app/graphql/types/merge_request_state_enum.rb +++ b/app/graphql/types/merge_request_state_enum.rb @@ -1,10 +1,13 @@ # frozen_string_literal: true module Types + # rubocop: disable Graphql/AuthorizeTypes + # This is a BaseEnum through IssuableEnum, so it does not need authorization class MergeRequestStateEnum < IssuableStateEnum graphql_name 'MergeRequestState' description 'State of a GitLab merge request' value 'merged' end + # rubocop: enable Graphql/AuthorizeTypes end diff --git a/app/graphql/types/metadata_type.rb b/app/graphql/types/metadata_type.rb index 2d8bad0614b..7d7813a7652 100644 --- a/app/graphql/types/metadata_type.rb +++ b/app/graphql/types/metadata_type.rb @@ -4,6 +4,8 @@ module Types class MetadataType < ::Types::BaseObject graphql_name 'Metadata' + authorize :read_instance_metadata + field :version, GraphQL::STRING_TYPE, null: false field :revision, GraphQL::STRING_TYPE, null: false end diff --git a/app/graphql/types/namespace_type.rb b/app/graphql/types/namespace_type.rb index 62feccaa660..f105e9e6e28 100644 --- a/app/graphql/types/namespace_type.rb +++ b/app/graphql/types/namespace_type.rb @@ -4,6 +4,8 @@ module Types class NamespaceType < BaseObject graphql_name 'Namespace' + authorize :read_namespace + field :id, GraphQL::ID_TYPE, null: false field :name, GraphQL::STRING_TYPE, null: false diff --git a/app/graphql/types/notes/diff_position_type.rb b/app/graphql/types/notes/diff_position_type.rb index 104ccb79bbb..ebc24451715 100644 --- a/app/graphql/types/notes/diff_position_type.rb +++ b/app/graphql/types/notes/diff_position_type.rb @@ -2,6 +2,8 @@ module Types module Notes + # rubocop: disable Graphql/AuthorizeTypes + # This is presented through `NoteType` that has its own authorization class DiffPositionType < BaseObject graphql_name 'DiffPosition' @@ -42,5 +44,6 @@ module Types description: "The total height of the image", resolve: -> (position, _args, _ctx) { position.height if position.on_image? } end + # rubocop: enable Graphql/AuthorizeTypes end end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index ac957eafafc..41fdc76b1e4 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -67,7 +67,7 @@ module Types field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true - field :namespace, Types::NamespaceType, null: false + field :namespace, Types::NamespaceType, null: true field :group, Types::GroupType, null: true field :statistics, Types::ProjectStatisticsType, diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 536bdb077ad..53d36b43576 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -22,10 +22,7 @@ module Types field :metadata, Types::MetadataType, null: true, resolver: Resolvers::MetadataResolver, - description: 'Metadata about GitLab' do |*args| - - authorize :read_instance_metadata - end + description: 'Metadata about GitLab' field :echo, GraphQL::STRING_TYPE, null: false, function: Functions::Echo.new end diff --git a/app/graphql/types/task_completion_status.rb b/app/graphql/types/task_completion_status.rb index c289802509d..ac128481ac4 100644 --- a/app/graphql/types/task_completion_status.rb +++ b/app/graphql/types/task_completion_status.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true module Types + # rubocop: disable Graphql/AuthorizeTypes + # This is used in `IssueType` and `MergeRequestType` both of which have their + # own authorization class TaskCompletionStatus < BaseObject graphql_name 'TaskCompletionStatus' description 'Completion status of tasks' @@ -8,4 +11,5 @@ module Types field :count, GraphQL::INT_TYPE, null: false field :completed_count, GraphQL::INT_TYPE, null: false end + # rubocop: enable Graphql/AuthorizeTypes end diff --git a/app/graphql/types/tree/blob_type.rb b/app/graphql/types/tree/blob_type.rb index 760781f3612..9497e378dc0 100644 --- a/app/graphql/types/tree/blob_type.rb +++ b/app/graphql/types/tree/blob_type.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Types module Tree + # rubocop: disable Graphql/AuthorizeTypes + # This is presented through `Repository` that has its own authorization class BlobType < BaseObject implements Types::Tree::EntryType @@ -12,6 +14,7 @@ module Types field :lfs_oid, GraphQL::STRING_TYPE, null: true, resolve: -> (blob, args, ctx) do Gitlab::Graphql::Loaders::BatchLfsOidLoader.new(blob.repository, blob.id).find end + # rubocop: enable Graphql/AuthorizeTypes end end end diff --git a/app/graphql/types/tree/submodule_type.rb b/app/graphql/types/tree/submodule_type.rb index cea76dbfd2a..8cb1e04f5ba 100644 --- a/app/graphql/types/tree/submodule_type.rb +++ b/app/graphql/types/tree/submodule_type.rb @@ -1,10 +1,13 @@ # frozen_string_literal: true module Types module Tree + # rubocop: disable Graphql/AuthorizeTypes + # This is presented through `Repository` that has its own authorization class SubmoduleType < BaseObject implements Types::Tree::EntryType graphql_name 'Submodule' end + # rubocop: enable Graphql/AuthorizeTypes end end diff --git a/app/graphql/types/tree/tree_entry_type.rb b/app/graphql/types/tree/tree_entry_type.rb index 23ec2ef0ec2..d7faa633706 100644 --- a/app/graphql/types/tree/tree_entry_type.rb +++ b/app/graphql/types/tree/tree_entry_type.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Types module Tree + # rubocop: disable Graphql/AuthorizeTypes + # This is presented through `Repository` that has its own authorization class TreeEntryType < BaseObject implements Types::Tree::EntryType @@ -11,5 +13,6 @@ module Types field :web_url, GraphQL::STRING_TYPE, null: true end + # rubocop: enable Graphql/AuthorizeTypes end end diff --git a/app/graphql/types/tree/tree_type.rb b/app/graphql/types/tree/tree_type.rb index 1ee93ed9542..e7644b071d3 100644 --- a/app/graphql/types/tree/tree_type.rb +++ b/app/graphql/types/tree/tree_type.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Types module Tree + # rubocop: disable Graphql/AuthorizeTypes + # This is presented through `Repository` that has its own authorization class TreeType < BaseObject graphql_name 'Tree' @@ -13,6 +15,7 @@ module Types field :blobs, Types::Tree::BlobType.connection_type, null: false, resolve: -> (obj, args, ctx) do Gitlab::Graphql::Representation::TreeEntry.decorate(obj.blobs, obj.repository) end + # rubocop: enable Graphql/AuthorizeTypes end end end diff --git a/spec/graphql/types/label_type_spec.rb b/spec/graphql/types/label_type_spec.rb index f498b32f9ed..8e7b2c69eff 100644 --- a/spec/graphql/types/label_type_spec.rb +++ b/spec/graphql/types/label_type_spec.rb @@ -7,4 +7,6 @@ describe GitlabSchema.types['Label'] do is_expected.to have_graphql_fields(*expected_fields) end + + it { is_expected.to require_graphql_authorizations(:read_label) } end diff --git a/spec/graphql/types/metadata_type_spec.rb b/spec/graphql/types/metadata_type_spec.rb index 55205bf5b6a..5236380e477 100644 --- a/spec/graphql/types/metadata_type_spec.rb +++ b/spec/graphql/types/metadata_type_spec.rb @@ -2,4 +2,5 @@ require 'spec_helper' describe GitlabSchema.types['Metadata'] do it { expect(described_class.graphql_name).to eq('Metadata') } + it { is_expected.to require_graphql_authorizations(:read_instance_metadata) } end diff --git a/spec/graphql/types/namespace_type_spec.rb b/spec/graphql/types/namespace_type_spec.rb index 77fd590586e..e1153832cc9 100644 --- a/spec/graphql/types/namespace_type_spec.rb +++ b/spec/graphql/types/namespace_type_spec.rb @@ -13,4 +13,6 @@ describe GitlabSchema.types['Namespace'] do is_expected.to have_graphql_fields(*expected_fields) end + + it { is_expected.to require_graphql_authorizations(:read_namespace) } end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index af1972a2513..bc3b8a42392 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -34,9 +34,5 @@ describe GitlabSchema.types['Query'] do is_expected.to have_graphql_type(Types::MetadataType) is_expected.to have_graphql_resolver(Resolvers::MetadataResolver) end - - it 'authorizes with read_instance_metadata' do - is_expected.to require_graphql_authorizations(:read_instance_metadata) - end end end diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index de1cd9586b6..63fa16c79ca 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -58,9 +58,7 @@ describe 'getting projects', :nested_groups do it 'finds only public projects' do post_graphql(query, current_user: nil) - expect(graphql_data['namespace']['projects']['edges'].size).to eq(1) - project = graphql_data['namespace']['projects']['edges'][0]['node'] - expect(project['id']).to eq(public_project.to_global_id.to_s) + expect(graphql_data['namespace']).to be_nil end end end -- GitLab