From 7a3374bbc6f4c798a2528ec1daf9095d2c23c024 Mon Sep 17 00:00:00 2001
From: Luke Bennett <lbennett@gitlab.com>
Date: Thu, 21 Mar 2019 20:28:41 +0000
Subject: [PATCH 1/3] Destroy repo mirrors instead of disabling them

It is important to destroy data related to repo mirrors when they are
disabled.
Use `_destroy` nested attribute instead of `enabled` for push mirrors.
Call `remove_import_data` after saving a project if its pull mirror is
disabled.
---
 .../javascripts/mirrors/mirror_repos.js       |  2 +-
 .../projects/mirrors_controller.rb            |  1 +
 ...ce-its-simple-just-destroy-the-mirrors.yml |  5 +++++
 ee/app/services/ee/projects/update_service.rb |  1 +
 .../mirrors/_mirror_repos_form.html.haml      |  2 +-
 .../its-simple-just-destroy-the-mirrors.yml   |  5 +++++
 .../ee/repository_mirrors_settings_spec.rb    | 15 +++++++++++++++
 .../services/projects/update_service_spec.rb  |  7 +++++++
 .../settings/repository_settings_spec.rb      | 19 +++++++++++++++++++
 9 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml
 create mode 100644 ee/changelogs/unreleased/its-simple-just-destroy-the-mirrors.yml

diff --git a/app/assets/javascripts/mirrors/mirror_repos.js b/app/assets/javascripts/mirrors/mirror_repos.js
index 196b84621b6547..33e9b1c4e46a83 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/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb
index f5942428355156..0e621c2062a72e 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/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 00000000000000..9bcc646d91e71d
--- /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: 26444
+author:
+type: security
diff --git a/ee/app/services/ee/projects/update_service.rb b/ee/app/services/ee/projects/update_service.rb
index 3e403813d09124..587a33b4b6820c 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 944203c53d713d..c95b240ca2010e 100644
--- a/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml
+++ b/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml
@@ -21,7 +21,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/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 00000000000000..039ef5600d7ef6
--- /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/settings/ee/repository_mirrors_settings_spec.rb b/ee/spec/features/projects/settings/ee/repository_mirrors_settings_spec.rb
index b06c96df3c344c..27ebc9ca42ce15 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 28e49ffc978418..10be8c4c356ea5 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/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb
index 1259ad45791395..f7de769cee9f70 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
-- 
GitLab


From 9c41f7ebb36018d7cbedc0e784e34424427d8e0d Mon Sep 17 00:00:00 2001
From: Luke Bennett <lukeeeebennettplus@gmail.com>
Date: Fri, 5 Apr 2019 05:23:42 +0100
Subject: [PATCH 2/3] Remove SSH public key before mirror is saved

Mirror credentials are now destroyed if there is no active mirror.
The means we cannot show a public key before the mirror is saved
because it will be deleted and recreated before/when it is saved.
The public SSH key is now displayed only after the mirror is saved.
---
 app/assets/javascripts/mirrors/ssh_mirror.js  | 114 +-----------------
 app/views/layouts/_flash.html.haml            |   3 +-
 .../mirrors/_authentication_method.html.haml  |  21 ----
 .../mirrors/_mirror_repos_push.html.haml      |   2 +-
 .../projects/mirrors/_ssh_host_keys.html.haml |   2 +-
 .../ee/projects/mirrors_controller.rb         |   1 +
 .../mirrors/_mirror_repos_form.html.haml      |  15 ++-
 ee/spec/features/projects/mirror_spec.rb      |  29 +----
 locale/gitlab.pot                             |   3 -
 9 files changed, 22 insertions(+), 168 deletions(-)

diff --git a/app/assets/javascripts/mirrors/ssh_mirror.js b/app/assets/javascripts/mirrors/ssh_mirror.js
index f7e809508039f6..bb5ae6ce2d1bd5 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/views/layouts/_flash.html.haml b/app/views/layouts/_flash.html.haml
index 2cdaa85bdaa2f0..dab6f5e8725d44 100644
--- a/app/views/layouts/_flash.html.haml
+++ b/app/views/layouts/_flash.html.haml
@@ -3,8 +3,7 @@
 .flash-container.flash-container-page
   -# We currently only support `alert`, `notice`, `success`
   - flash.each do |key, value|
-    -# Don't show a flash message if the message is nil
-    - if value
+    - if value.is_a? String
       %div{ class: "flash-#{key}" }
         %div{ class: "#{(container_class unless fluid_layout)} #{(extra_flash_class unless @no_container)} #{@content_class}" }
           %span= value
diff --git a/app/views/projects/mirrors/_authentication_method.html.haml b/app/views/projects/mirrors/_authentication_method.html.haml
index ef6db07a1bb3b5..ee82d68d398c34 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 1d9c83653feba2..b7c885b4a63fc1 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 f61aa6ecd11103..7762fb4b8449fd 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/ee/app/controllers/ee/projects/mirrors_controller.rb b/ee/app/controllers/ee/projects/mirrors_controller.rb
index 4e3727447c918d..86022aaa6ce05c 100644
--- a/ee/app/controllers/ee/projects/mirrors_controller.rb
+++ b/ee/app/controllers/ee/projects/mirrors_controller.rb
@@ -11,6 +11,7 @@ def update
         result = ::Projects::UpdateService.new(project, current_user, safe_mirror_params).execute
 
         if result[:status] == :success
+          flash[:ssh_public_key] = true
           flash[:notice] =
             if project.mirror?
               _('Mirroring settings were successfully updated. The project is being updated.')
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 c95b240ca2010e..b56be5a28cc480 100644
--- a/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml
+++ b/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml
@@ -1,18 +1,27 @@
 - 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
 
+- public_key = @project.import_data.ssh_public_key
+- if flash[:ssh_public_key] && public_key
+  .js-ssh-public-key
+    .mt-3.mb-1.d-flex
+      %code.ssh-public-key= public_key
+      = clipboard_button(text: public_key, title: _("Copy SSH public key to clipboard"), class: 'btn-copy-ssh-public-key mb-auto')
+    .mb-3= _('Here is the public SSH key that needs to be added to the remote server. For more information, please refer to the documentation.')
+
 %template.js-push-mirrors-form
   = render partial: "projects/mirrors/mirror_repos_push", locals: { f: f }
 
diff --git a/ee/spec/features/projects/mirror_spec.rb b/ee/spec/features/projects/mirror_spec.rb
index eda0561e2cd14a..ff661e8407327f 100644
--- a/ee/spec/features/projects/mirror_spec.rb
+++ b/ee/spec/features/projects/mirror_spec.rb
@@ -142,41 +142,22 @@
           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(page).to have_content(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
+        refresh
 
-        expect(page).not_to have_content(first_key)
-        expect(page).to have_content(import_data.reload.ssh_public_key)
+        expect(page).to have_selector('.mirror-url')
+        expect(page).not_to have_content(import_data.ssh_public_key)
       end
     end
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8c2ff348b00d95..90bc7d48b8670f 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 ""
 
-- 
GitLab


From af32393d1baa5be7673d5e52aeda7977bb25a44e Mon Sep 17 00:00:00 2001
From: Luke Bennett <lbennett@gitlab.com>
Date: Sun, 7 Apr 2019 17:40:39 +0100
Subject: [PATCH 3/3] Add clipboard copy btn to pull ssh public keys

Push mirrors have a copy to clipboard button for their
public ssh keys, this commit adds it to pull mirror rows.
---
 app/views/layouts/_flash.html.haml            |  3 +-
 ...ce-its-simple-just-destroy-the-mirrors.yml |  2 +-
 .../ee/projects/mirrors_controller.rb         |  1 -
 .../mirrors/_mirror_repos_form.html.haml      |  8 ------
 .../mirrors/_table_pull_row.html.haml         |  3 ++
 ee/spec/features/projects/mirror_spec.rb      | 28 +------------------
 locale/gitlab.pot                             |  9 ------
 7 files changed, 7 insertions(+), 47 deletions(-)

diff --git a/app/views/layouts/_flash.html.haml b/app/views/layouts/_flash.html.haml
index dab6f5e8725d44..2cdaa85bdaa2f0 100644
--- a/app/views/layouts/_flash.html.haml
+++ b/app/views/layouts/_flash.html.haml
@@ -3,7 +3,8 @@
 .flash-container.flash-container-page
   -# We currently only support `alert`, `notice`, `success`
   - flash.each do |key, value|
-    - if value.is_a? String
+    -# Don't show a flash message if the message is nil
+    - if value
       %div{ class: "flash-#{key}" }
         %div{ class: "#{(container_class unless fluid_layout)} #{(extra_flash_class unless @no_container)} #{@content_class}" }
           %span= value
diff --git a/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml b/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml
index 9bcc646d91e71d..ac5fc27cf36e50 100644
--- a/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml
+++ b/changelogs/unreleased/ce-its-simple-just-destroy-the-mirrors.yml
@@ -1,5 +1,5 @@
 ---
 title: Destroy project remote mirrors instead of disabling
-merge_request: 26444
+merge_request: 27087
 author:
 type: security
diff --git a/ee/app/controllers/ee/projects/mirrors_controller.rb b/ee/app/controllers/ee/projects/mirrors_controller.rb
index 86022aaa6ce05c..4e3727447c918d 100644
--- a/ee/app/controllers/ee/projects/mirrors_controller.rb
+++ b/ee/app/controllers/ee/projects/mirrors_controller.rb
@@ -11,7 +11,6 @@ def update
         result = ::Projects::UpdateService.new(project, current_user, safe_mirror_params).execute
 
         if result[:status] == :success
-          flash[:ssh_public_key] = true
           flash[:notice] =
             if project.mirror?
               _('Mirroring settings were successfully updated. The project is being updated.')
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 b56be5a28cc480..db6ca0c16584f8 100644
--- a/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml
+++ b/ee/app/views/projects/mirrors/_mirror_repos_form.html.haml
@@ -14,14 +14,6 @@
 
 .js-form-insertion-point
 
-- public_key = @project.import_data.ssh_public_key
-- if flash[:ssh_public_key] && public_key
-  .js-ssh-public-key
-    .mt-3.mb-1.d-flex
-      %code.ssh-public-key= public_key
-      = clipboard_button(text: public_key, title: _("Copy SSH public key to clipboard"), class: 'btn-copy-ssh-public-key mb-auto')
-    .mb-3= _('Here is the public SSH key that needs to be added to the remote server. For more information, please refer to the documentation.')
-
 %template.js-push-mirrors-form
   = render partial: "projects/mirrors/mirror_repos_push", locals: { f: f }
 
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 dc472f33a6dc23..c06a2c61c04892 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/spec/features/projects/mirror_spec.rb b/ee/spec/features/projects/mirror_spec.rb
index ff661e8407327f..8d62abd3792d34 100644
--- a/ee/spec/features/projects/mirror_spec.rb
+++ b/ee/spec/features/projects/mirror_spec.rb
@@ -148,16 +148,11 @@
 
         expect(page).to have_content('Mirroring settings were successfully updated')
         expect(page).not_to have_content('Verified by')
-        expect(page).to have_content(import_data.ssh_public_key)
+        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
-
-        refresh
-
-        expect(page).to have_selector('.mirror-url')
-        expect(page).not_to have_content(import_data.ssh_public_key)
       end
     end
 
@@ -185,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/locale/gitlab.pot b/locale/gitlab.pot
index 90bc7d48b8670f..ce5f048570663b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3387,9 +3387,6 @@ msgstr ""
 msgid "Copy SSH public key"
 msgstr ""
 
-msgid "Copy SSH public key to clipboard"
-msgstr ""
-
 msgid "Copy URL to clipboard"
 msgstr ""
 
@@ -6286,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 ""
 
@@ -13064,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 ""
 
-- 
GitLab