Merge branch 'security-disable-mirroring-fix' into 'master'

Restrict mirroring changes to admins only when mirroring is disabled

See merge request gitlab-org/security/gitlab!275

app/controllers/projects/mirrors_controller.rb

@@ -67,7 +67,7 @@ class Projects::MirrorsController < Projects::ApplicationController
   end
 
   def check_mirror_available!
-    Gitlab::CurrentSettings.current_application_settings.mirror_available || current_user&.admin?
+    render_404 unless can?(current_user, :admin_remote_mirror, project)
   end
 
   def mirror_params_attributes

app/views/projects/mirrors/_mirror_repos.html.haml

 - expanded = expanded_by_default?
 - protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|')
+- mirror_settings_enabled = can?(current_user, :admin_remote_mirror, @project)
+- mirror_settings_class = "#{'expanded' if expanded} #{'js-mirror-settings' if mirror_settings_enabled}".strip
 
-%section.settings.project-mirror-settings.js-mirror-settings.no-animate#js-push-remote-settings{ class: ('expanded' if expanded), data: { qa_selector: 'mirroring_repositories_settings_section' } }
+%section.settings.project-mirror-settings.no-animate#js-push-remote-settings{ class: mirror_settings_class, data: { qa_selector: 'mirroring_repositories_settings_section' } }
   .settings-header
     %h4= _('Mirroring repositories')
     %button.btn.js-settings-toggle
@@ -11,26 +13,32 @@
       = link_to _('Read more'), help_page_path('workflow/repository_mirroring'), target: '_blank'
 
   .settings-content
-    = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f|
-      .panel.panel-default
-        .panel-body
-          %div= form_errors(@project)
+    - if mirror_settings_enabled
+      = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f|
+        .panel.panel-default
+          .panel-body
+            %div= form_errors(@project)
 
-          .form-group.has-feedback
-            = label_tag :url, _('Git repository URL'), class: 'label-light'
-            = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password'
+            .form-group.has-feedback
+              = label_tag :url, _('Git repository URL'), class: 'label-light'
+              = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password'
 
-          = render 'projects/mirrors/instructions'
+            = render 'projects/mirrors/instructions'
 
-          = render 'projects/mirrors/mirror_repos_form', f: f
+            = render 'projects/mirrors/mirror_repos_form', f: f
 
-          .form-check.append-bottom-10
-            = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input'
-            = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label'
-            = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank'
+            .form-check.append-bottom-10
+              = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input'
+              = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label'
+              = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank'
 
-        .panel-footer
-          = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror
+          .panel-footer
+            = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror
+    - else
+      .gl-alert.gl-alert-info{ role: 'alert' }
+        = sprite_icon('information-o', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+        .gl-alert-body
+          = _('Mirror settings are only available to GitLab administrators.')
 
     .panel.panel-default
       .table-responsive
@@ -61,8 +69,10 @@
                   - if mirror.last_error.present?
                     .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true', qa_selector: 'mirror_error_badge' }, title: html_escape(mirror.last_error.try(:strip)) }= _('Error')
                 %td
-                  .btn-group.mirror-actions-group.pull-right{ role: 'group' }
-                    - if mirror.ssh_key_auth?
-                      = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button')
-                    = render 'shared/remote_mirror_update_button', remote_mirror: mirror
+                  - if mirror_settings_enabled
+                    .btn-group.mirror-actions-group.pull-right{ role: 'group' }
+                      - if mirror.ssh_key_auth?
+                        = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button')
+                      = render 'shared/remote_mirror_update_button', remote_mirror: mirror
                     %button.js-delete-mirror.qa-delete-mirror.rspec-delete-mirror.btn.btn-danger{ type: 'button', data: { mirror_id: mirror.id, toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o')
+

changelogs/unreleased/security-disable-mirroring-fix.yml

+---
+title: Restrict mirroring changes to admins only when mirroring is disabled
+merge_request:
+author:
+type: security

locale/gitlab.pot

@@ -12801,6 +12801,9 @@ msgstr ""
 msgid "Mirror repository"
 msgstr ""
 
+msgid "Mirror settings are only available to GitLab administrators."
+msgstr ""
+
 msgid "Mirror user"
 msgstr ""
 

spec/controllers/projects/mirrors_controller_spec.rb

@@ -5,6 +5,72 @@ require 'spec_helper'
 describe Projects::MirrorsController do
   include ReactiveCachingHelpers
 
+  shared_examples 'only admin is allowed when mirroring is disabled' do
+    let(:subject_action) { raise 'subject_action is required' }
+    let(:user) { project.owner }
+    let(:project_settings_path) { project_settings_repository_path(project, anchor: 'js-push-remote-settings') }
+
+    context 'when project mirroring is enabled' do
+      it 'allows requests from a maintainer' do
+        sign_in(user)
+
+        subject_action
+        expect(response).to redirect_to(project_settings_path)
+      end
+
+      it 'allows requests from an admin user' do
+        user.update!(admin: true)
+        sign_in(user)
+
+        subject_action
+        expect(response).to redirect_to(project_settings_path)
+      end
+    end
+
+    context 'when project mirroring is disabled' do
+      before do
+        stub_application_setting(mirror_available: false)
+      end
+
+      it 'disallows requests from a maintainer' do
+        sign_in(user)
+
+        subject_action
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      it 'allows requests from an admin user' do
+        user.update!(admin: true)
+        sign_in(user)
+
+        subject_action
+        expect(response).to redirect_to(project_settings_path)
+      end
+    end
+  end
+
+  describe 'Access control' do
+    let(:project) { create(:project, :repository) }
+
+    describe '#update' do
+      include_examples 'only admin is allowed when mirroring is disabled' do
+        let(:subject_action) do
+          do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } })
+        end
+      end
+    end
+
+    describe '#update_now' do
+      include_examples 'only admin is allowed when mirroring is disabled' do
+        let(:options) { { namespace_id: project.namespace, project_id: project } }
+
+        let(:subject_action) do
+          get :update_now, params: options.merge(sync_remote: true)
+        end
+      end
+    end
+  end
+
   describe 'setting up a remote mirror' do
     let_it_be(:project) { create(:project, :repository) }
 

spec/features/projects/settings/repository_settings_spec.rb

@@ -26,11 +26,7 @@ describe 'Projects > Settings > Repository settings' do
     let(:role) { :maintainer }
 
     context 'remote mirror settings' do
-      let(:user2) { create(:user) }
-
       before do
-        project.add_maintainer(user2)
-
         visit project_settings_repository_path(project)
       end
 
@@ -90,6 +86,18 @@ describe 'Projects > Settings > Repository settings' do
         expect(page).to have_selector('[title="Copy SSH public key"]')
       end
 
+      context 'when project mirroring is disabled' do
+        before do
+          stub_application_setting(mirror_available: false)
+          visit project_settings_repository_path(project)
+        end
+
+        it 'hides remote mirror settings' do
+          expect(page.find('.project-mirror-settings')).not_to have_selector('form')
+          expect(page).to have_content('Mirror settings are only available to GitLab administrators.')
+        end
+      end
+
       def select_direction(direction = 'push')
         direction_select = find('#mirror_direction')
 
@@ -154,4 +162,31 @@ describe 'Projects > Settings > Repository settings' do
       expect(mirror).not_to have_selector('.rspec-update-now-button')
     end
   end
+
+  context 'for admin' do
+    shared_examples_for 'shows mirror settings' do
+      it 'shows mirror settings' do
+        expect(page.find('.project-mirror-settings')).to have_selector('form')
+        expect(page).not_to have_content('Changing mirroring setting is disabled for non-admin users.')
+      end
+    end
+
+    before do
+      stub_application_setting(mirror_available: mirror_available)
+      user.update!(admin: true)
+      visit project_settings_repository_path(project)
+    end
+
+    context 'when project mirroring is enabled' do
+      let(:mirror_available) { true }
+
+      include_examples 'shows mirror settings'
+    end
+
+    context 'when project mirroring is disabled' do
+      let(:mirror_available) { false }
+
+      include_examples 'shows mirror settings'
+    end
+  end
 end