diff --git a/app/assets/javascripts/mirrors/mirror_repos.js b/app/assets/javascripts/mirrors/mirror_repos.js index 196b84621b65473fe0764ee5ea01e2fab4538ba4..33e9b1c4e46a83a4824608511cc9f2b881d5e532 100644 --- a/app/assets/javascripts/mirrors/mirror_repos.js +++ b/app/assets/javascripts/mirrors/mirror_repos.js @@ -87,7 +87,7 @@ export default class MirrorRepos { project: { remote_mirrors_attributes: { id: $target.data('mirrorId'), - enabled: 0, + _destroy: 1, }, }, }; diff --git a/app/assets/javascripts/mirrors/ssh_mirror.js b/app/assets/javascripts/mirrors/ssh_mirror.js index f7e809508039f6ca5f55eaecd49ba8560466d811..bb5ae6ce2d1bd54001bd9dc4106a788c766e49d8 100644 --- a/app/assets/javascripts/mirrors/ssh_mirror.js +++ b/app/assets/javascripts/mirrors/ssh_mirror.js @@ -24,12 +24,6 @@ export default class SSHMirror { this.$wellAuthTypeChanging = this.$form.find('.js-well-changing-auth'); this.$wellPasswordAuth = this.$form.find('.js-well-password-auth'); - this.$wellSSHAuth = this.$form.find('.js-well-ssh-auth'); - this.$sshPublicKeyWrap = this.$form.find('.js-ssh-public-key-wrap'); - this.$regeneratePublicSshKeyButton = this.$wellSSHAuth.find('.js-btn-regenerate-ssh-key'); - this.$regeneratePublicSshKeyModal = this.$wellSSHAuth.find( - '.js-regenerate-public-ssh-key-confirm-modal', - ); } init() { @@ -40,15 +34,6 @@ export default class SSHMirror { this.$dropdownAuthType.on('change', e => this.handleAuthTypeChange(e)); this.$btnDetectHostKeys.on('click', e => this.handleDetectHostKeys(e)); this.$btnSSHHostsShowAdvanced.on('click', e => this.handleSSHHostsAdvanced(e)); - this.$regeneratePublicSshKeyButton.on('click', () => - this.$regeneratePublicSshKeyModal.toggle(true), - ); - $('.js-confirm', this.$regeneratePublicSshKeyModal).on('click', e => - this.regeneratePublicSshKey(e), - ); - $('.js-cancel', this.$regeneratePublicSshKeyModal).on('click', () => - this.$regeneratePublicSshKeyModal.toggle(false), - ); } /** @@ -162,54 +147,11 @@ export default class SSHMirror { * Authentication method dropdown change event listener */ handleAuthTypeChange() { - const projectMirrorAuthTypeEndpoint = `${this.$form.attr('action')}.json`; - const $sshPublicKey = this.$sshPublicKeyWrap.find('.ssh-public-key'); const selectedAuthType = this.$dropdownAuthType.val(); this.$wellPasswordAuth.collapse('hide'); - this.$wellSSHAuth.collapse('hide'); this.updateHiddenAuthType(selectedAuthType); - - // This request should happen only if selected Auth type was SSH - // and SSH Public key was not present on page load - if (selectedAuthType === AUTH_METHOD.SSH && !$sshPublicKey.text().trim()) { - if (!this.$wellSSHAuth.length) return; - - // Construct request body - const authTypeData = { - project: { - ...this.$regeneratePublicSshKeyButton.data().projectData, - }, - }; - - this.$wellAuthTypeChanging.collapse('show'); - this.$dropdownAuthType.disable(); - - axios - .put(projectMirrorAuthTypeEndpoint, JSON.stringify(authTypeData), { - headers: { - 'Content-Type': 'application/json; charset=utf-8', - }, - }) - .then(({ data }) => { - // Show SSH public key container and fill in public key - this.toggleAuthWell(selectedAuthType); - this.toggleSSHAuthWellMessage(true); - this.setSSHPublicKey(data.import_data_attributes.ssh_public_key); - - this.$wellAuthTypeChanging.collapse('hide'); - this.$dropdownAuthType.enable(); - }) - .catch(() => { - Flash(__('Something went wrong on our end.')); - - this.$wellAuthTypeChanging.collapse('hide'); - this.$dropdownAuthType.enable(); - }); - } else { - this.toggleAuthWell(selectedAuthType); - this.$wellSSHAuth.find('.js-ssh-public-key-present').collapse('show'); - } + this.toggleAuthWell(selectedAuthType); } /** @@ -235,7 +177,6 @@ export default class SSHMirror { */ toggleAuthWell(authType) { this.$wellPasswordAuth.collapse(authType === AUTH_METHOD.PASSWORD ? 'show' : 'hide'); - this.$wellSSHAuth.collapse(authType === AUTH_METHOD.SSH ? 'show' : 'hide'); this.updateHiddenAuthType(authType); } @@ -244,64 +185,11 @@ export default class SSHMirror { this.$hiddenAuthType.prop('disabled', authType === AUTH_METHOD.SSH); } - /** - * Toggle SSH auth information message - */ - toggleSSHAuthWellMessage(sshKeyPresent) { - this.$sshPublicKeyWrap.collapse(sshKeyPresent ? 'show' : 'hide'); - this.$wellSSHAuth.find('.js-ssh-public-key-present').collapse(sshKeyPresent ? 'show' : 'hide'); - this.$regeneratePublicSshKeyButton.collapse(sshKeyPresent ? 'show' : 'hide'); - this.$wellSSHAuth.find('.js-ssh-public-key-pending').collapse(sshKeyPresent ? 'hide' : 'show'); - } - - /** - * Sets SSH Public key to Clipboard button and shows it on UI. - */ - setSSHPublicKey(sshPublicKey) { - this.$sshPublicKeyWrap.find('.ssh-public-key').text(sshPublicKey); - this.$sshPublicKeyWrap - .find('.btn-copy-ssh-public-key') - .attr('data-clipboard-text', sshPublicKey); - } - - regeneratePublicSshKey(event) { - event.preventDefault(); - - this.$regeneratePublicSshKeyModal.toggle(false); - - const button = this.$regeneratePublicSshKeyButton; - const spinner = $('.js-spinner', button); - const endpoint = button.data('endpoint'); - const authTypeData = { - project: { - ...this.$regeneratePublicSshKeyButton.data().projectData, - }, - }; - - button.attr('disabled', 'disabled'); - spinner.removeClass('d-none'); - - axios - .patch(endpoint, authTypeData) - .then(({ data }) => { - button.removeAttr('disabled'); - spinner.addClass('d-none'); - - this.setSSHPublicKey(data.import_data_attributes.ssh_public_key); - }) - .catch(() => { - Flash(__('Unable to regenerate public ssh key.')); - }); - } - destroy() { this.$repositoryUrl.off('keyup'); this.$form.find('.js-known-hosts').off('keyup'); this.$dropdownAuthType.off('change'); this.$btnDetectHostKeys.off('click'); this.$btnSSHHostsShowAdvanced.off('click'); - this.$regeneratePublicSshKeyButton.off('click'); - $('.js-confirm', this.$regeneratePublicSshKeyModal).off('click'); - $('.js-cancel', this.$regeneratePublicSshKeyModal).off('click'); } } diff --git a/app/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb index f5942428355156563845e9648856b53ab8047c3c..0e621c2062a72ec978d221751148fbb479f3f135 100644 --- a/app/controllers/projects/mirrors_controller.rb +++ b/app/controllers/projects/mirrors_controller.rb @@ -81,6 +81,7 @@ def mirror_params_attributes password ssh_known_hosts regenerate_ssh_private_key + _destroy ] ] end diff --git a/app/views/projects/mirrors/_authentication_method.html.haml b/app/views/projects/mirrors/_authentication_method.html.haml index ef6db07a1bb3b57a36216e26ed36d71f82322da2..ee82d68d398c346ae8918ec216971b1b6753ae57 100644 --- a/app/views/projects/mirrors/_authentication_method.html.haml +++ b/app/views/projects/mirrors/_authentication_method.html.haml @@ -1,8 +1,5 @@ - mirror = f.object -- is_push = local_assigns.fetch(:is_push, false) - auth_options = [[_('Password'), 'password'], [_('SSH public key'), 'ssh_public_key']] -- regen_data = { auth_method: 'ssh_public_key', regenerate_ssh_private_key: true } -- ssh_public_key_present = mirror.ssh_public_key.present? .form-group = f.label :auth_method, _('Authentication method'), class: 'label-bold' @@ -17,21 +14,3 @@ .well-password-auth.collapse.js-well-password-auth = f.label :password, _("Password"), class: "label-bold" = f.password_field :password, value: mirror.password, class: 'form-control qa-password', autocomplete: 'new-password' - - unless is_push - .well-ssh-auth.collapse.js-well-ssh-auth - %p.js-ssh-public-key-present{ class: ('collapse' unless ssh_public_key_present) } - = _('Here is the public SSH key that needs to be added to the remote server. For more information, please refer to the documentation.') - %p.js-ssh-public-key-pending{ class: ('collapse' if ssh_public_key_present) } - = _('An SSH key will be automatically generated when the form is submitted. For more information, please refer to the documentation.') - - .clearfix.js-ssh-public-key-wrap{ class: ('collapse' unless ssh_public_key_present) } - %code.prepend-top-10.ssh-public-key - = mirror.ssh_public_key - = clipboard_button(text: mirror.ssh_public_key, title: _("Copy SSH public key to clipboard"), class: 'prepend-top-10 btn-copy-ssh-public-key') - - = button_tag type: 'button', - data: { endpoint: project_mirror_path(@project), project_data: { import_data_attributes: regen_data } }, - class: "btn btn-inverted btn-warning prepend-top-10 js-btn-regenerate-ssh-key#{ ' collapse' unless ssh_public_key_present }" do - = icon('spinner spin', class: 'js-spinner d-none') - = _('Regenerate key') - = render 'projects/mirrors/regenerate_public_ssh_key_confirm_modal' diff --git a/app/views/projects/mirrors/_mirror_repos_push.html.haml b/app/views/projects/mirrors/_mirror_repos_push.html.haml index 1d9c83653feba29c96f90914ad4afbb4e5c786de..b7c885b4a63fc12d28c30161d8d3a564e4bb0ee6 100644 --- a/app/views/projects/mirrors/_mirror_repos_push.html.haml +++ b/app/views/projects/mirrors/_mirror_repos_push.html.haml @@ -5,4 +5,4 @@ = rm_f.hidden_field :url, class: 'js-mirror-url-hidden', required: true, pattern: "(#{protocols}):\/\/.+" = rm_f.hidden_field :only_protected_branches, class: 'js-mirror-protected-hidden' = render partial: 'projects/mirrors/ssh_host_keys', locals: { f: rm_f } - = render partial: 'projects/mirrors/authentication_method', locals: { f: rm_f, is_push: true } + = render partial: 'projects/mirrors/authentication_method', locals: { f: rm_f } diff --git a/app/views/projects/mirrors/_ssh_host_keys.html.haml b/app/views/projects/mirrors/_ssh_host_keys.html.haml index f61aa6ecd1110385ee1fb28e892458de4f5227d2..7762fb4b8449fd79ce270238b823e0d46742e2f0 100644 --- a/app/views/projects/mirrors/_ssh_host_keys.html.haml +++ b/app/views/projects/mirrors/_ssh_host_keys.html.haml @@ -3,7 +3,7 @@ - verified_at = mirror.ssh_known_hosts_verified_at .form-group.js-ssh-host-keys-section{ class: ('collapse' unless mirror.ssh_mirror_url?) } - %button.btn.btn-inverted.btn-success.inline.js-detect-host-keys.append-right-10{ type: 'button' } + %button.btn.btn-inverted.btn-secondary.inline.js-detect-host-keys.append-right-10{ type: 'button' } = icon('spinner spin', class: 'js-spinner d-none') = _('Detect host keys') .fingerprint-ssh-info.js-fingerprint-ssh-info.prepend-top-10.append-bottom-10{ class: ('collapse' unless mirror.ssh_mirror_url?) } diff --git a/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml b/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml new file mode 100644 index 0000000000000000000000000000000000000000..ac5fc27cf36e501a3041614ad9e44182511b53f9 --- /dev/null +++ b/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml @@ -0,0 +1,5 @@ +--- +title: Destroy project remote mirrors instead of disabling +merge_request: 27087 +author: +type: security diff --git a/ee/app/services/ee/projects/update_service.rb b/ee/app/services/ee/projects/update_service.rb index 3e403813d091247cb79eac4ec4cc7dfcad0a287c..587a33b4b6820c0ea81fc5324908c9054f5d6533 100644 --- a/ee/app/services/ee/projects/update_service.rb +++ b/ee/app/services/ee/projects/update_service.rb @@ -34,6 +34,7 @@ def execute sync_wiki_on_enable if !wiki_was_enabled && project.wiki_enabled? project.import_state.force_import_job! if params[:mirror].present? && project.mirror? + project.remove_import_data if project.previous_changes.include?('mirror') && !project.mirror? sync_approval_rules end diff --git a/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml b/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml index 944203c53d713ded2dd8c8d7a7de478336f94a6f..db6ca0c16584f827b8f8422a125e1496b7554849 100644 --- a/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml +++ b/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml @@ -1,15 +1,16 @@ - import_data = @project.import_data || @project.build_import_data - is_one_user_option = default_mirror_users.count == 1 - protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|') -- options = [[_('Push'), 'push']] +- direction_options = [[_('Push'), 'push']] - has_existing_pull_mirror = @project.mirror.present? + - if can?(current_user, :admin_mirror, @project) - - pull_addition_method = has_existing_pull_mirror ? options.method(:push) : options.method(:unshift) + - pull_addition_method = has_existing_pull_mirror ? direction_options.method(:push) : direction_options.method(:unshift) - pull_addition_method.call([_('Pull'), 'pull']) .form-group = label_tag :mirror_direction, _('Mirror direction'), class: 'label-light' - = select_tag :mirror_direction, options_for_select(options), class: 'form-control js-mirror-direction qa-mirror-direction', disabled: (options.count == 1) || has_existing_pull_mirror + = select_tag :mirror_direction, options_for_select(direction_options), class: 'form-control js-mirror-direction qa-mirror-direction', disabled: (direction_options.count == 1) || has_existing_pull_mirror .js-form-insertion-point @@ -21,7 +22,7 @@ = f.hidden_field :username_only_import_url, class: 'js-mirror-url-hidden', required: true, pattern: "(#{protocols}):\/\/.+" = f.hidden_field :only_mirror_protected_branches, class: 'js-mirror-protected-hidden' - = f.fields_for :import_data, import_data do |import_form| + = f.fields_for :import_data, import_data, include_id: false do |import_form| = render partial: 'projects/mirrors/ssh_host_keys', locals: { f: import_form } = render partial: 'projects/mirrors/authentication_method', locals: { f: import_form } diff --git a/ee/app/views/projects/mirrors/_table_pull_row.html.haml b/ee/app/views/projects/mirrors/_table_pull_row.html.haml index dc472f33a6dc2395b2f846b4b88b46fc3b7cf29c..c06a2c61c048922d12577f675ded32f1aac4fdd0 100644 --- a/ee/app/views/projects/mirrors/_table_pull_row.html.haml +++ b/ee/app/views/projects/mirrors/_table_pull_row.html.haml @@ -12,6 +12,9 @@ %td.mirror-action-buttons .btn-group.mirror-actions-group.pull-right{ role: 'group' } - if @project.mirror? + - ssh_public_key = @project.import_data.ssh_public_key + - if ssh_public_key + = clipboard_button(text: ssh_public_key, class: 'btn btn-default qa-copy-ssh-public-key', title: _('Copy SSH public key')) - if import_state.mirror_update_due? || import_state.updating_mirror? %button.btn.disabled{ type: 'button', data: { container: 'body', toggle: 'tooltip' }, title: _('Updating') }= icon("refresh spin") - else diff --git a/ee/changelogs/unreleased/its-simple-just-destroy-the-mirrors.yml b/ee/changelogs/unreleased/its-simple-just-destroy-the-mirrors.yml new file mode 100644 index 0000000000000000000000000000000000000000..039ef5600d7ef6a87768ced95e11687afd1e7889 --- /dev/null +++ b/ee/changelogs/unreleased/its-simple-just-destroy-the-mirrors.yml @@ -0,0 +1,5 @@ +--- +title: Destroy project remote pull mirrors instead of disabling +merge_request: 10355 +author: +type: security diff --git a/ee/spec/features/projects/mirror_spec.rb b/ee/spec/features/projects/mirror_spec.rb index eda0561e2cd14afae6d63f25924139dc45e1de10..8d62abd3792d34faccc35f3aaefc13e9b8319ef2 100644 --- a/ee/spec/features/projects/mirror_spec.rb +++ b/ee/spec/features/projects/mirror_spec.rb @@ -142,41 +142,17 @@ select('Pull', from: 'Mirror direction') select 'SSH public key', from: 'Authentication method' - # Generates an SSH public key with an asynchronous PUT and displays it - wait_for_requests - - expect(import_data.ssh_public_key).not_to be_nil - expect(page).to have_content(import_data.ssh_public_key) - click_without_sidekiq 'Mirror repository' end + project.reload - # We didn't set any host keys expect(page).to have_content('Mirroring settings were successfully updated') expect(page).not_to have_content('Verified by') - - project.reload - import_data.reload + expect(find('.qa-copy-ssh-public-key')['data-clipboard-text']).to eq(import_data.ssh_public_key) expect(project.mirror?).to be_truthy expect(project.username_only_import_url).to eq('ssh://user@example.com') expect(import_data.auth_method).to eq('ssh_public_key') expect(import_data.password).to be_blank - - find('.js-delete-mirror').click - fill_in 'Git repository URL', with: 'ssh://user@example.com' - select('Pull', from: 'Mirror direction') - - first_key = import_data.ssh_public_key - expect(page).to have_content(first_key) - - # Check regenerating the public key works - click_without_sidekiq 'Regenerate key' - find('.js-regenerate-public-ssh-key-confirm-modal .js-confirm').click - - wait_for_requests - - expect(page).not_to have_content(first_key) - expect(page).to have_content(import_data.reload.ssh_public_key) end end @@ -204,27 +180,6 @@ end end - it 'preserves the existing SSH key after generating it once' do - stub_reactive_cache(cache, known_hosts: key.key_text) - - visit project_settings_repository_path(project) - - page.within('.project-mirror-settings') do - fill_in 'Git repository URL', with: 'ssh://example.com' - select('Pull', from: 'Mirror direction') - select 'SSH public key', from: 'Authentication method' - click_on 'Detect host keys' - - wait_for_requests - - expect(page).to have_content(key.fingerprint) - - wait_for_requests - - expect { click_on 'Mirror repository' }.not_to change { import_data.reload.ssh_public_key } - end - end - it 'displays error if detection fails' do stub_reactive_cache(cache, error: 'Some error text here') diff --git a/ee/spec/features/projects/settings/ee/repository_mirrors_settings_spec.rb b/ee/spec/features/projects/settings/ee/repository_mirrors_settings_spec.rb index b06c96df3c344c3b2d6164d08d8b80eeb3b75777..27ebc9ca42ce1589af209aee69fbbfbb879c2b26 100644 --- a/ee/spec/features/projects/settings/ee/repository_mirrors_settings_spec.rb +++ b/ee/spec/features/projects/settings/ee/repository_mirrors_settings_spec.rb @@ -66,5 +66,20 @@ expect(mirror_url).to include('https://*****@github.com/') end end + + context 'with an existing pull mirror', :js do + let(:mirrored_project) { create(:project, :repository, :mirror, namespace: user.namespace) } + + it 'deletes the mirror' do + visit project_settings_repository_path(mirrored_project) + + find('.js-delete-mirror').click + wait_for_requests + mirrored_project.reload + + expect(mirrored_project.import_data).to be_nil + expect(mirrored_project).not_to be_mirror + end + end end end diff --git a/ee/spec/services/projects/update_service_spec.rb b/ee/spec/services/projects/update_service_spec.rb index 28e49ffc978418e6cc7e36060887404341fe4c93..10be8c4c356ea52a52feaa3e5fda03ab985b5d2e 100644 --- a/ee/spec/services/projects/update_service_spec.rb +++ b/ee/spec/services/projects/update_service_spec.rb @@ -277,6 +277,13 @@ expect(result).to eq({ status: :error, message: "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'." }) end + it 'calls remove_import_data if mirror was disabled in previous change' do + update_project(project, user, { mirror: false }) + + expect(project.import_data).to be_nil + expect(project).not_to be_mirror + end + def update_project(project, user, opts) Projects::UpdateService.new(project, user, opts).execute end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8c2ff348b00d951d4f08761391b6ba1a12d986cb..ce5f048570663be735ea73a146a84700dfeae650 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -971,9 +971,6 @@ msgstr "" msgid "Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication" msgstr "" -msgid "An SSH key will be automatically generated when the form is submitted. For more information, please refer to the documentation." -msgstr "" - msgid "An application called %{link_to_client} is requesting access to your GitLab account." msgstr "" @@ -3390,9 +3387,6 @@ msgstr "" msgid "Copy SSH public key" msgstr "" -msgid "Copy SSH public key to clipboard" -msgstr "" - msgid "Copy URL to clipboard" msgstr "" @@ -6289,9 +6283,6 @@ msgstr "" msgid "Help page text and support page url." msgstr "" -msgid "Here is the public SSH key that needs to be added to the remote server. For more information, please refer to the documentation." -msgstr "" - msgid "Hide file browser" msgstr "" @@ -13067,9 +13058,6 @@ msgstr "" msgid "Unable to load the diff. %{button_try_again}" msgstr "" -msgid "Unable to regenerate public ssh key." -msgstr "" - msgid "Unable to resolve" msgstr "" diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 1259ad4579139516fbd64ddab2d845a3fb9d536c..f7de769cee9f70fe0c4796c10e64980e91e5d25c 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -217,5 +217,24 @@ def select_direction(direction = 'push') expect(RepositoryCleanupWorker.jobs.count).to eq(1) end end + + context 'with an existing mirror', :js do + let(:mirrored_project) { create(:project, :repository, :remote_mirror) } + + before do + mirrored_project.add_maintainer(user) + + visit project_settings_repository_path(mirrored_project) + end + + it 'delete remote mirrors' do + expect(mirrored_project.remote_mirrors.count).to eq(1) + + find('.js-delete-mirror').click + wait_for_requests + + expect(mirrored_project.remote_mirrors.count).to eq(0) + end + end end end