Merge remote-tracking branch 'dev/master'

.gitlab/ci/releases.gitlab-ci.yml

@@ -9,7 +9,7 @@
   image: alpine:edge
   stage: sync
   before_script:
-    - apk add --no-cache --update curl bash
+    - apk add --no-cache --update curl bash jq
   after_script: []
   script:
     - bash scripts/sync-stable-branch.sh

CHANGELOG-EE.md

 Please view this file on the master branch, on stable branches it's out of date.
 
+## 12.6.2
+
+### Security (2 changes)
+
+- Don't publish drafts if user can't create notes.
+- Remove protected tag access when group is removed.
+
+
 ## 12.6.1
 
 - No changes.

CHANGELOG.md

@@ -2,6 +2,18 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.6.2
+
+### Security (6 changes)
+
+- GraphQL: Add timeout to all queries.
+- Filter out notification settings for projects that a user does not have at least read access.
+- Hide project name and path when unsusbcribing from an issue or merge request.
+- Fix 500 error caused by invalid byte sequences in uploads links.
+- Return only runners from groups where user is owner for user CI owned runners.
+- Fix Vulnerability of Release Evidence.
+
+
 ## 12.6.1
 
 ### Fixed (2 changes)

app/controllers/profiles/notifications_controller.rb

@@ -11,6 +11,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController
       exclude_group_ids: @group_notifications.select(:source_id)
     ).execute.map { |group| current_user.notification_settings_for(group, inherit: true) }
     @project_notifications = current_user.notification_settings.for_projects.order(:id)
+                             .select { |notification| current_user.can?(:read_project, notification.source) }
     @global_notification_setting = current_user.global_notification_setting
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/controllers/projects/releases_controller.rb

@@ -10,7 +10,7 @@ class Projects::ReleasesController < Projects::ApplicationController
     push_frontend_feature_flag(:release_evidence_collection, project)
   end
   before_action :authorize_update_release!, only: %i[edit update]
-  before_action :authorize_download_code!, only: [:evidence]
+  before_action :authorize_read_release_evidence!, only: [:evidence]
 
   def index
     respond_to do |format|
@@ -47,6 +47,11 @@ class Projects::ReleasesController < Projects::ApplicationController
     access_denied! unless can?(current_user, :update_release, release)
   end
 
+  def authorize_read_release_evidence!
+    access_denied! unless Feature.enabled?(:release_evidence, project, default_enabled: true)
+    access_denied! unless can?(current_user, :read_release_evidence, release)
+  end
+
   def release
     @release ||= project.releases.find_by_tag!(sanitized_tag_name)
   end

app/helpers/notifications_helper.rb

@@ -116,4 +116,8 @@ module NotificationsHelper
   def show_unsubscribe_title?(noteable)
     can?(current_user, "read_#{noteable.to_ability_name}".to_sym, noteable)
   end
+
+  def can_read_project?(project)
+    can?(current_user, :read_project, project)
+  end
 end

app/models/evidence.rb

@@ -15,6 +15,21 @@ class Evidence < ApplicationRecord
     @milestones ||= release.milestones.includes(:issues)
   end
 
+  ##
+  # Return `summary` without sensitive information.
+  #
+  # Removing issues from summary in order to prevent leaking confidential ones.
+  # See more https://gitlab.com/gitlab-org/gitlab/issues/121930
+  def summary
+    safe_summary = read_attribute(:summary)
+
+    safe_summary.dig('release', 'milestones')&.each do |milestone|
+      milestone.delete('issues')
+    end
+
+    safe_summary
+  end
+
   private
 
   def generate_summary_and_sha

app/models/user.rb

@@ -1327,7 +1327,7 @@ class User < ApplicationRecord
         .select('ci_runners.*')
 
       group_runners = Ci::RunnerNamespace
-        .where(namespace_id: owned_or_maintainers_groups.select(:id))
+        .where(namespace_id: owned_groups.select(:id))
         .joins(:runner)
         .select('ci_runners.*')
 

app/policies/release_policy.rb

@@ -2,4 +2,31 @@
 
 class ReleasePolicy < BasePolicy
   delegate { @subject.project }
+
+  rule { allowed_to_read_evidence & external_authorization_service_disabled }.policy do
+    enable :read_release_evidence
+  end
+
+  ##
+  # evidence.summary includes the following entities:
+  # - Release
+  # - git-tag (Repository)
+  # - Project
+  # - Milestones
+  # - Issues
+  condition(:allowed_to_read_evidence) do
+    can?(:read_release) &&
+      can?(:download_code) &&
+      can?(:read_project) &&
+      can?(:read_milestone) &&
+      can?(:read_issue)
+  end
+
+  ##
+  # Currently, we don't support release evidence for the GitLab instances
+  # that enables external authorization services.
+  # See https://gitlab.com/gitlab-org/gitlab/issues/121930.
+  condition(:external_authorization_service_disabled) do
+    !Gitlab::ExternalAuthorization::Config.enabled?
+  end
 end

app/views/sent_notifications/unsubscribe.html.haml

 - noteable = @sent_notification.noteable
 - noteable_type = @sent_notification.noteable_type.titleize.downcase
 - noteable_text = show_unsubscribe_title?(noteable) ? %(#{noteable.title} (#{noteable.to_reference})) : %(#{noteable.to_reference})
-- page_title _("Unsubscribe"), noteable_text, noteable_type.pluralize, @sent_notification.project.full_name
+- show_project_path = can_read_project?(@sent_notification.project)
+- project_path = show_project_path ? @sent_notification.project.full_name : _("GitLab / Unsubscribe")
+- noteable_url = show_project_path ? url_for([@sent_notification.project.namespace.becomes(Namespace), @sent_notification.project, noteable]) : breadcrumb_title_link
+- page_title _('Unsubscribe'), noteable_text, noteable_type.pluralize, project_path
 
 %h3.page-title
   = _("Unsubscribe from %{type}") % { type: noteable_type }
 
 %p
-  - link_to_noteable_text = link_to(noteable_text, url_for([@sent_notification.project.namespace.becomes(Namespace), @sent_notification.project, noteable]))
+  - link_to_noteable_text = link_to(noteable_text, noteable_url)
   = _("Are you sure you want to unsubscribe from the %{type}: %{link_to_noteable_text}?").html_safe % { type: noteable_type, link_to_noteable_text: link_to_noteable_text }
 
 %p

config/initializers/graphql.rb

@@ -5,3 +5,7 @@ GraphQL::Field.accepts_definitions(authorize: GraphQL::Define.assign_metadata_ke
 
 GraphQL::Schema::Object.accepts_definition(:authorize)
 GraphQL::Schema::Field.accepts_definition(:authorize)
+
+GitlabSchema.middleware << GraphQL::Schema::TimeoutMiddleware.new(max_seconds: ENV.fetch('GITLAB_RAILS_GRAPHQL_TIMEOUT', 30).to_i) do |timeout_error, query|
+  Gitlab::GraphqlLogger.error(message: timeout_error.to_s, query: query.query_string, query_variables: query.provided_variables)
+end

ee/app/controllers/projects/merge_requests/drafts_controller.rb

@@ -6,7 +6,7 @@ class Projects::MergeRequests::DraftsController < Projects::MergeRequests::Appli
   respond_to :json
 
   before_action :check_draft_notes_available!, except: [:index]
-  before_action :authorize_create_draft!, only: [:create]
+  before_action :authorize_create_note!, only: [:create, :publish]
   before_action :authorize_admin_draft!, only: [:update, :destroy]
   before_action :authorize_admin_draft!, if: -> { action_name == 'publish' && params[:id].present? }
 
@@ -128,7 +128,7 @@ class Projects::MergeRequests::DraftsController < Projects::MergeRequests::Appli
     access_denied! unless can?(current_user, :admin_note, draft_note)
   end
 
-  def authorize_create_draft!
+  def authorize_create_note!
     access_denied! unless can?(current_user, :create_note, merge_request)
   end
 

ee/app/models/ee/project_group_link.rb

@@ -15,6 +15,9 @@ module EE
       project.protected_branches.merge_access_by_group(group).destroy_all # rubocop: disable DestroyAll
       project.protected_branches.push_access_by_group(group).destroy_all # rubocop: disable DestroyAll
 
+      # For protected tags
+      project.protected_tags.create_access_by_group(group).delete_all
+
       # For protected environments
       project.protected_environments.deploy_access_levels_by_group(group).delete_all
     end

ee/app/services/draft_notes/publish_service.rb

@@ -3,6 +3,8 @@
 module DraftNotes
   class PublishService < DraftNotes::BaseService
     def execute(draft = nil)
+      return error('Not allowed to create notes') unless can?(current_user, :create_note, merge_request)
+
       if draft
         publish_draft_note(draft)
       else

ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb

@@ -215,18 +215,42 @@ describe Projects::MergeRequests::DraftsController do
 
   describe 'POST #publish' do
     context 'without permissions' do
+      shared_examples_for 'action that does not allow publishing draft note' do
+        it 'does not allow publishing draft note' do
+          expect { action }
+            .to not_change { Note.count }
+            .and not_change { DraftNote.count }
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
       before do
         sign_in(user2)
-        project.add_developer(user2)
       end
 
-      it 'does not allow publishing draft note belonging to someone else' do
-        draft = create(:draft_note, merge_request: merge_request, author: user)
+      context 'when note belongs to someone else' do
+        before do
+          project.add_developer(user2)
+        end
 
-        expect { post :publish, params: params.merge(id: draft.id) }.to change { Note.count }.by(0)
-          .and change { DraftNote.count }.by(0)
+        it_behaves_like 'action that does not allow publishing draft note' do
+          let!(:draft) { create(:draft_note, merge_request: merge_request, author: user) }
+          let(:action) { post :publish, params: params.merge(id: draft.id) }
+        end
+      end
 
-        expect(response).to have_gitlab_http_status(404)
+      context 'when merge request discussion is locked' do
+        let(:project) { create(:project, :public, :merge_requests_public, :repository) }
+
+        before do
+          create(:draft_note, merge_request: merge_request, author: user2)
+          merge_request.update!(discussion_locked: true)
+        end
+
+        it_behaves_like 'action that does not allow publishing draft note' do
+          let(:action) { post :publish, params: params }
+        end
       end
     end
 

ee/spec/models/ee/project_group_link_spec.rb

@@ -13,16 +13,46 @@ describe ProjectGroupLink do
       project.add_developer(user)
     end
 
-    it 'removes related protected environment deploy access levels' do
-      params = attributes_for(:protected_environment,
-                              deploy_access_levels_attributes: [{ group_id: group.id }, { user_id: user.id }])
+    shared_examples_for 'deleted related access levels' do |access_level_class|
+      it "removes related #{access_level_class}" do
+        expect { project_group_link.destroy! }.to change(access_level_class, :count).by(-1)
+        expect(access_levels.find_by_group_id(group)).to be_nil
+        expect(access_levels.find_by_user_id(user)).to be_persisted
+      end
+    end
+
+    context 'protected tags' do
+      let!(:protected_tag) do
+        ProtectedTags::CreateService.new(
+          project,
+          project.owner,
+          attributes_for(
+            :protected_tag,
+            create_access_levels_attributes: [{ group_id: group.id }, { user_id: user.id }]
+          )
+        ).execute
+      end
+
+      let(:access_levels) { protected_tag.create_access_levels }
+
+      it_behaves_like 'deleted related access levels', ProtectedTag::CreateAccessLevel
+    end
 
-      protected_environment = ProtectedEnvironments::CreateService.new(project, user, params).execute
+    context 'protected environments' do
+      let!(:protected_environment) do
+        ProtectedEnvironments::CreateService.new(
+          project,
+          project.owner,
+          attributes_for(
+            :protected_environment,
+            deploy_access_levels_attributes: [{ group_id: group.id }, { user_id: user.id }]
+          )
+        ).execute
+      end
 
-      expect { project_group_link.destroy! }.to change(ProtectedEnvironment::DeployAccessLevel, :count).by(-1)
+      let(:access_levels) { protected_environment.deploy_access_levels }
 
-      expect(protected_environment.deploy_access_levels.find_by_group_id(group)).to be_nil
-      expect(protected_environment.deploy_access_levels.find_by_user_id(user)).to be_persisted
+      it_behaves_like 'deleted related access levels', ProtectedEnvironment::DeployAccessLevel
     end
   end
 end

ee/spec/services/draft_notes/publish_service_spec.rb

@@ -246,4 +246,14 @@ describe DraftNotes::PublishService do
       publish
     end
   end
+
+  context 'user cannot create notes' do
+    before do
+      allow(Ability).to receive(:allowed?).with(user, :create_note, merge_request).and_return(false)
+    end
+
+    it 'returns an error' do
+      expect(publish[:status]).to eq(:error)
+    end
+  end
 end

lib/api/entities.rb

@@ -1363,7 +1363,7 @@ module API
       expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? }
       expose :commit, using: Entities::Commit, if: ->(_, _) { can_download_code? }
       expose :upcoming_release?, as: :upcoming_release
-      expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? }
+      expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? && can_read_milestone? }
       expose :commit_path, expose_nil: false
       expose :tag_path, expose_nil: false
       expose :evidence_sha, expose_nil: false, if: ->(_, _) { can_download_code? }
@@ -1389,6 +1389,10 @@ module API
       def can_download_code?
         Ability.allowed?(options[:current_user], :download_code, object.project)
       end
+
+      def can_read_milestone?
+        Ability.allowed?(options[:current_user], :read_milestone, object.project)
+      end
     end
 
     class Tag < Grape::Entity

lib/banzai/filter/relative_link_filter.rb

@@ -116,7 +116,7 @@ module Banzai
       end
 
       def process_link_to_upload_attr(html_attr)
-        path_parts = [Addressable::URI.unescape(html_attr.value)]
+        path_parts = [unescape_and_scrub_uri(html_attr.value)]
 
         if project
           path_parts.unshift(relative_url_root, project.full_path)
@@ -172,7 +172,7 @@ module Banzai
       end
 
       def cleaned_file_path(uri)
-        Addressable::URI.unescape(uri.path).scrub.delete("\0").chomp("/")
+        unescape_and_scrub_uri(uri.path).delete("\0").chomp("/")
       end
 
       def relative_file_path(uri)
@@ -184,7 +184,7 @@ module Banzai
       def request_path
         return unless context[:requested_path]
 
-        Addressable::URI.unescape(context[:requested_path]).chomp("/")
+        unescape_and_scrub_uri(context[:requested_path]).chomp("/")
       end
 
       # Convert a relative path into its correct location based on the currently
@@ -266,6 +266,12 @@ module Banzai
       def repository
         @repository ||= project&.repository
       end
+
+      private
+
+      def unescape_and_scrub_uri(uri)
+        Addressable::URI.unescape(uri).scrub
+      end
     end
   end
 end

locale/gitlab.pot

@@ -8579,6 +8579,9 @@ msgstr ""
 msgid "GitHub import"
 msgstr ""
 
+msgid "GitLab / Unsubscribe"
+msgstr ""
+
 msgid "GitLab CI Linter has been moved"
 msgstr ""
 

scripts/sync-stable-branch.sh

@@ -35,6 +35,22 @@ then
     exit 1
 fi
 
+if [[ "$TARGET_PROJECT" != "gitlab-org/gitlab-foss" ]]
+then
+    echo 'This is a security FOSS merge train'
+    echo "Checking if $CI_COMMIT_SHA is available on canonical"
+
+    gitlab_com_commit_status=$(curl -s "https://gitlab.com/api/v4/projects/278964/repository/commits/$CI_COMMIT_SHA" | jq -M .status)
+
+    if [[ "$gitlab_com_commit_status" != "null" ]]
+    then
+       echo 'Commit available on canonical, skipping merge train'
+       exit 0
+    fi
+
+    echo 'Commit not available, triggering a merge train'
+fi
+
 curl -X POST \
     -F token="$MERGE_TRAIN_TRIGGER_TOKEN" \
     -F ref=master \

spec/controllers/profiles/notifications_controller_spec.rb

@@ -52,6 +52,35 @@ describe Profiles::NotificationsController do
         end.to exceed_query_limit(control)
       end
     end
+
+    context 'with project notifications' do
+      let!(:notification_setting) { create(:notification_setting, source: project, user: user, level: :watch) }
+
+      before do
+        sign_in(user)
+        get :show
+      end
+
+      context 'when project is public' do
+        let(:project) { create(:project, :public) }
+
+        it 'shows notification setting for project' do
+          expect(assigns(:project_notifications).map(&:source_id)).to include(project.id)
+        end
+      end
+
+      context 'when project is public' do
+        let(:project) { create(:project, :private) }
+
+        it 'shows notification setting for project' do
+          # notification settings for given project were created before project was set to private
+          expect(user.notification_settings.for_projects.map(&:source_id)).to include(project.id)
+
+          # check that notification settings for project where user does not have access are filtered
+          expect(assigns(:project_notifications)).to be_empty
+        end
+      end
+    end
   end
 
   describe 'POST update' do

spec/controllers/projects/releases_controller_spec.rb

@@ -167,7 +167,7 @@ describe Projects::ReleasesController do
   end
 
   describe 'GET #evidence' do
-    let(:tag_name) { "v1.1.0-evidence" }
+    let_it_be(:tag_name) { "v1.1.0-evidence" }
     let!(:release) { create(:release, :with_evidence, project: project, tag: tag_name) }
     let(:tag) { CGI.escape(release.tag) }
     let(:format) { :json }
@@ -220,6 +220,85 @@ describe Projects::ReleasesController do
         it_behaves_like 'successful request'
       end
     end
+
+    context 'when release is associated to a milestone which includes an issue' do
+      let_it_be(:project) { create(:project, :repository, :public) }
+      let_it_be(:issue) { create(:issue, project: project) }
+      let_it_be(:milestone) { create(:milestone, project: project, issues: [issue]) }
+      let_it_be(:release) { create(:release, project: project, tag: tag_name, milestones: [milestone]) }
+
+      before do
+        create(:evidence, release: release)
+      end
+
+      shared_examples_for 'does not show the issue in evidence' do
+        it do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['release']['milestones']
+            .all? { |milestone| milestone['issues'].nil? }).to eq(true)
+        end
+      end
+
+      shared_examples_for 'evidence not found' do
+        it do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      shared_examples_for 'safely expose evidence' do
+        it_behaves_like 'does not show the issue in evidence'
+
+        context 'when the issue is confidential' do
+          let(:issue) { create(:issue, :confidential, project: project) }
+
+          it_behaves_like 'does not show the issue in evidence'
+        end
+
+        context 'when the user is the author of the confidential issue' do
+          let(:issue) { create(:issue, :confidential, project: project, author: user) }
+
+          it_behaves_like 'does not show the issue in evidence'
+        end
+
+        context 'when project is private' do
+          let!(:project) { create(:project, :repository, :private) }
+
+          it_behaves_like 'evidence not found'
+        end
+
+        context 'when project restricts the visibility of issues to project members only' do
+          let!(:project) { create(:project, :repository, :issues_private) }
+
+          it_behaves_like 'evidence not found'
+        end
+      end
+
+      context 'when user is non-project member' do
+        let(:user) { create(:user) }
+
+        it_behaves_like 'safely expose evidence'
+      end
+
+      context 'when user is auditor', if: Gitlab.ee? do
+        let(:user) { create(:user, :auditor) }
+
+        it_behaves_like 'safely expose evidence'
+      end
+
+      context 'when external authorization control is enabled' do
+        let(:user) { create(:user) }
+
+        before do
+          stub_application_setting(external_authorization_service_enabled: true)
+        end
+
+        it_behaves_like 'evidence not found'
+      end
+    end
   end
 
   private

spec/controllers/sent_notifications_controller_spec.rb

@@ -56,7 +56,7 @@ describe SentNotificationsController do
           get(:unsubscribe, params: { id: sent_notification.reply_key })
         end
 
-        shared_examples 'unsubscribing as anonymous' do
+        shared_examples 'unsubscribing as anonymous' do |project_visibility|
           it 'does not unsubscribe the user' do
             expect(noteable.subscribed?(user, target_project)).to be_truthy
           end
@@ -69,6 +69,18 @@ describe SentNotificationsController do
             expect(response.status).to eq(200)
             expect(response).to render_template :unsubscribe
           end
+
+          if project_visibility == :private
+            it 'does not show project name or path' do
+              expect(response.body).not_to include(noteable.project.name)
+              expect(response.body).not_to include(noteable.project.full_name)
+            end
+          else
+            it 'shows project name or path' do
+              expect(response.body).to include(noteable.project.name)
+              expect(response.body).to include(noteable.project.full_name)
+            end
+          end
         end
 
         context 'when project is public' do
@@ -79,7 +91,7 @@ describe SentNotificationsController do
               expect(response.body).to include(issue.title)
             end
 
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :public
           end
 
           context 'when unsubscribing from confidential issue' do
@@ -90,7 +102,7 @@ describe SentNotificationsController do
               expect(response.body).to include(confidential_issue.to_reference)
             end
 
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :public
           end
 
           context 'when unsubscribing from merge request' do
@@ -100,7 +112,12 @@ describe SentNotificationsController do
               expect(response.body).to include(merge_request.title)
             end
 
-            it_behaves_like 'unsubscribing as anonymous'
+            it 'shows project name or path' do
+              expect(response.body).to include(issue.project.name)
+              expect(response.body).to include(issue.project.full_name)
+            end
+
+            it_behaves_like 'unsubscribing as anonymous', :public
           end
         end
 
@@ -110,11 +127,11 @@ describe SentNotificationsController do
           context 'when unsubscribing from issue' do
             let(:noteable) { issue }
 
-            it 'shows issue title' do
+            it 'does not show issue title' do
               expect(response.body).not_to include(issue.title)
             end
 
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :private
           end
 
           context 'when unsubscribing from confidential issue' do
@@ -125,17 +142,17 @@ describe SentNotificationsController do
               expect(response.body).to include(confidential_issue.to_reference)
             end
 
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :private
           end
 
           context 'when unsubscribing from merge request' do
             let(:noteable) { merge_request }
 
-            it 'shows merge request title' do
+            it 'dos not show merge request title' do
               expect(response.body).not_to include(merge_request.title)
             end
 
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :private
           end
         end
       end

spec/fixtures/api/schemas/evidences/milestone.json

@@ -7,8 +7,7 @@
     "state",
     "iid",
     "created_at",
-    "due_date",
-    "issues"
+    "due_date"
   ],
   "properties": {
     "id": { "type": "integer" },
@@ -17,11 +16,7 @@
     "state": { "type": "string" },
     "iid": { "type": "integer" },
     "created_at": { "type": "date" },
-    "due_date": { "type": ["date", "null"] },
-    "issues": {
-      "type": "array",
-      "items": { "$ref": "issue.json" }
-    }
+    "due_date": { "type": ["date", "null"] }
   },
   "additionalProperties": false
 }

spec/lib/banzai/filter/relative_link_filter_spec.rb

@@ -128,6 +128,15 @@ describe Banzai::Filter::RelativeLinkFilter do
     expect { filter(act) }.not_to raise_error
   end
 
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences in uploads' do
+    act = link("/uploads/%FF")
+    expect { filter(act) }.not_to raise_error
+  end
+
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences in context requested path' do
+    expect { filter(link("files/test.md"), requested_path: '%FF') }.not_to raise_error
+  end
+
   it 'does not raise an exception with a garbled path' do
     act = link("open(/var/tmp/):%20/location%0Afrom:%20/test")
     expect { filter(act) }.not_to raise_error

spec/models/user_spec.rb

@@ -2638,8 +2638,8 @@ describe User, :do_not_mock_admin_mode do
           add_user(:maintainer)
         end
 
-        it 'loads' do
-          expect(user.ci_owned_runners).to contain_exactly(runner)
+        it 'does not load' do
+          expect(user.ci_owned_runners).to be_empty
         end
       end
 
@@ -2654,6 +2654,20 @@ describe User, :do_not_mock_admin_mode do
       end
     end
 
+    shared_examples :group_member do
+      context 'when the user is owner' do
+        before do
+          add_user(:owner)
+        end
+
+        it 'loads' do
+          expect(user.ci_owned_runners).to contain_exactly(runner)
+        end
+      end
+
+      it_behaves_like :member
+    end
+
     context 'with groups projects runners' do
       let(:group) { create(:group) }
       let!(:project) { create(:project, group: group) }
@@ -2662,7 +2676,7 @@ describe User, :do_not_mock_admin_mode do
         group.add_user(user, access)
       end
 
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
 
     context 'with groups runners' do
@@ -2673,14 +2687,14 @@ describe User, :do_not_mock_admin_mode do
         group.add_user(user, access)
       end
 
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
 
     context 'with other projects runners' do
       let!(:project) { create(:project) }
 
       def add_user(access)
-        project.add_role(user, access)
+        project.add_user(user, access)
       end
 
       it_behaves_like :member
@@ -2698,7 +2712,7 @@ describe User, :do_not_mock_admin_mode do
         subgroup.add_user(another_user, :owner)
       end
 
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
   end
 

spec/requests/api/graphql/gitlab_schema_spec.rb

@@ -8,6 +8,18 @@ describe 'GitlabSchema configurations' do
   set(:project) { create(:project) }
 
   shared_examples 'imposing query limits' do
+    describe 'timeouts' do
+      context 'when timeout is reached' do
+        it 'shows an error' do
+          Timecop.scale(50000000) do # ludicrously large number because the timeout has to happen before the query even begins
+            subject
+
+            expect_graphql_errors_to_include /Timeout/
+          end
+        end
+      end
+    end
+
     describe '#max_complexity' do
       context 'when complexity is too high' do
         it 'shows an error' do

spec/requests/api/releases_spec.rb

@@ -340,6 +340,40 @@ describe API::Releases do
 
           expect(response).to have_gitlab_http_status(:ok)
         end
+
+        context 'when release is associated to a milestone' do
+          let!(:release) do
+            create(:release, tag: 'v0.1', project: project, milestones: [milestone])
+          end
+
+          let(:milestone) { create(:milestone, project: project) }
+
+          it 'exposes milestones' do
+            get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+            expect(json_response['milestones'].first['title']).to eq(milestone.title)
+          end
+
+          context 'when project restricts visibility of issues and merge requests' do
+            let!(:project) { create(:project, :repository, :public, :issues_private, :merge_requests_private) }
+
+            it 'does not expose milestones' do
+              get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+              expect(json_response['milestones']).to be_nil
+            end
+          end
+
+          context 'when project restricts visibility of issues' do
+            let!(:project) { create(:project, :repository, :public, :issues_private) }
+
+            it 'exposes milestones' do
+              get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+              expect(json_response['milestones'].first['title']).to eq(milestone.title)
+            end
+          end
+        end
       end
     end
   end

spec/requests/api/runners_spec.rb

@@ -6,6 +6,7 @@ describe API::Runners do
   let(:admin) { create(:user, :admin) }
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
+  let(:group_maintainer) { create(:user) }
 
   let(:project) { create(:project, creator_id: user.id) }
   let(:project2) { create(:project, creator_id: user.id) }
@@ -20,6 +21,7 @@ describe API::Runners do
 
   before do
     # Set project access for users
+    create(:group_member, :maintainer, user: group_maintainer, group: group)
     create(:project_member, :maintainer, user: user, project: project)
     create(:project_member, :maintainer, user: user, project: project2)
     create(:project_member, :reporter, user: user2, project: project)
@@ -525,6 +527,20 @@ describe API::Runners do
           end.to change { Ci::Runner.project_type.count }.by(-1)
         end
 
+        it 'does not delete group runner with maintainer access' do
+          delete api("/runners/#{group_runner.id}", group_maintainer)
+
+          expect(response).to have_http_status(403)
+        end
+
+        it 'deletes group runner with owner access' do
+          expect do
+            delete api("/runners/#{group_runner.id}", user)
+
+            expect(response).to have_http_status(204)
+          end.to change { Ci::Runner.group_type.count }.by(-1)
+        end
+
         it_behaves_like '412 response' do
           let(:request) { api("/runners/#{project_runner.id}", user) }
         end