Merge branch '46396-recognise-when-a-user-is-trying-to-validate-a-private-ssh-key' into 'master'

(Part 2) Resolve "Recognise when a user is trying to validate a private SSH key"

Closes #46396

See merge request gitlab-org/gitlab-ce!19997

7 files changed, 162 insertions, 3 deletions

diff --git a/app/assets/javascripts/pages/profiles/keys/index.js b/app/assets/javascripts/pages/profiles/keys/index.js
new file mode 100644
index 00000000000..1cd3ee1dfdb
--- /dev/null
+++ b/app/assets/javascripts/pages/profiles/keys/index.js
@@ -0,0 +1,16 @@
+import AddSshKeyValidation from '~/profile/add_ssh_key_validation';
+
+document.addEventListener('DOMContentLoaded', () => {
+  const input = document.querySelector('.js-add-ssh-key-validation-input');
+  const warning = document.querySelector('.js-add-ssh-key-validation-warning');
+  const originalSubmit = input.form.querySelector('.js-add-ssh-key-validation-original-submit');
+  const confirmSubmit = warning.querySelector('.js-add-ssh-key-validation-confirm-submit');
+
+  const addSshKeyValidation = new AddSshKeyValidation(
+    input,
+    warning,
+    originalSubmit,
+    confirmSubmit,
+  );
+  addSshKeyValidation.register();
+});
diff --git a/app/assets/javascripts/profile/add_ssh_key_validation.js b/app/assets/javascripts/profile/add_ssh_key_validation.js
new file mode 100644
index 00000000000..ab6a6c1896c
--- /dev/null
+++ b/app/assets/javascripts/profile/add_ssh_key_validation.js
@@ -0,0 +1,43 @@
+export default class AddSshKeyValidation {
+  constructor(inputElement, warningElement, originalSubmitElement, confirmSubmitElement) {
+    this.inputElement = inputElement;
+    this.form = inputElement.form;
+
+    this.warningElement = warningElement;
+
+    this.originalSubmitElement = originalSubmitElement;
+    this.confirmSubmitElement = confirmSubmitElement;
+
+    this.isValid = false;
+  }
+
+  register() {
+    this.form.addEventListener('submit', event => this.submit(event));
+
+    this.confirmSubmitElement.addEventListener('click', () => {
+      this.isValid = true;
+      this.form.submit();
+    });
+
+    this.inputElement.addEventListener('input', () => this.toggleWarning(false));
+  }
+
+  submit(event) {
+    this.isValid = AddSshKeyValidation.isPublicKey(this.inputElement.value);
+
+    if (this.isValid) return true;
+
+    event.preventDefault();
+    this.toggleWarning(true);
+    return false;
+  }
+
+  toggleWarning(isVisible) {
+    this.warningElement.classList.toggle('hide', !isVisible);
+    this.originalSubmitElement.classList.toggle('hide', isVisible);
+  }
+
+  static isPublicKey(value) {
+    return /^(ssh|ecdsa-sha2)-/.test(value);
+  }
+}
diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss
index 282e424fc38..a22454c24e2 100644
--- a/app/assets/stylesheets/framework/forms.scss
+++ b/app/assets/stylesheets/framework/forms.scss
@@ -255,3 +255,8 @@ label {
     color: $theme-gray-600;
   }
 }
+
+.input-lg {
+  max-width: 320px;
+  width: 100%;
+}
diff --git a/app/views/profiles/keys/_form.html.haml b/app/views/profiles/keys/_form.html.haml
index c14700794ce..43a2d53b84d 100644
--- a/app/views/profiles/keys/_form.html.haml
+++ b/app/views/profiles/keys/_form.html.haml
@@ -5,11 +5,18 @@
     .form-group
       = f.label :key, class: 'label-light'
       %p= _("Paste your public SSH key, which is usually contained in the file '~/.ssh/id_rsa.pub' and begins with 'ssh-rsa'. Don't use your private SSH key.")
-      = f.text_area :key, class: "form-control", rows: 8, required: true, placeholder: 'Typically starts with "ssh-rsa …"'
+      = f.text_area :key, class: "form-control js-add-ssh-key-validation-input", rows: 8, required: true, placeholder: s_('Profiles|Typically starts with "ssh-rsa …"')
     .form-group
       = f.label :title, class: 'label-light'
-      = f.text_field :title, class: "form-control", required: true, placeholder: 'e.g. My MacBook key'
+      = f.text_field :title, class: "form-control input-lg", required: true, placeholder: s_('Profiles|e.g. My MacBook key')
       %p.form-text.text-muted= _('Name your individual key via a title')
 
+    .js-add-ssh-key-validation-warning.hide
+      .bs-callout.bs-callout-warning{ role: 'alert', aria_live: 'assertive' }
+        %strong= _('Oops, are you sure?')
+        %p= s_("Profiles|This doesn't look like a public SSH key, are you sure you want to add it?")
+
+      %button.btn.btn-create.js-add-ssh-key-validation-confirm-submit= _("Yes, add it")
+
     .prepend-top-default
-      = f.submit 'Add key', class: "btn btn-create"
+      = f.submit s_('Profiles|Add key'), class: "btn btn-create js-add-ssh-key-validation-original-submit"
diff --git a/changelogs/unreleased/46396-recognise-when-a-user-is-trying-to-validate-a-private-ssh-key.yml b/changelogs/unreleased/46396-recognise-when-a-user-is-trying-to-validate-a-private-ssh-key.yml
new file mode 100644
index 00000000000..64bbecf3405
--- /dev/null
+++ b/changelogs/unreleased/46396-recognise-when-a-user-is-trying-to-validate-a-private-ssh-key.yml
@@ -0,0 +1,5 @@
+---
+title: Update new SSH key page to improve key input validation
+merge_request: 19997
+author:
+type: other
diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb
index bfb17a56613..e6586fc8a0a 100644
--- a/spec/features/profiles/keys_spec.rb
+++ b/spec/features/profiles/keys_spec.rb
@@ -30,6 +30,20 @@ describe 'Profile > SSH Keys' do
       expect(find('.breadcrumbs-sub-title')).to have_link(attrs[:title])
     end
 
+    it 'shows a confirmable warning if the key does not start with ssh-' do
+      attrs = attributes_for(:key)
+
+      fill_in('Key', with: 'invalid-key')
+      fill_in('Title', with: attrs[:title])
+      click_button('Add key')
+
+      expect(page).to have_selector('.js-add-ssh-key-validation-warning')
+
+      find('.js-add-ssh-key-validation-confirm-submit').click
+
+      expect(page).to have_content('Key is invalid')
+    end
+
     context 'when only DSA and ECDSA keys are allowed' do
       before do
         forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE
diff --git a/spec/javascripts/profile/add_ssh_key_validation_spec.js b/spec/javascripts/profile/add_ssh_key_validation_spec.js
new file mode 100644
index 00000000000..c71a2885acc
--- /dev/null
+++ b/spec/javascripts/profile/add_ssh_key_validation_spec.js
@@ -0,0 +1,69 @@
+import AddSshKeyValidation from '../../../app/assets/javascripts/profile/add_ssh_key_validation';
+
+describe('AddSshKeyValidation', () => {
+  describe('submit', () => {
+    it('returns true if isValid is true', () => {
+      const addSshKeyValidation = new AddSshKeyValidation({});
+      spyOn(AddSshKeyValidation, 'isPublicKey').and.returnValue(true);
+
+      expect(addSshKeyValidation.submit()).toBeTruthy();
+    });
+
+    it('calls preventDefault and toggleWarning if isValid is false', () => {
+      const addSshKeyValidation = new AddSshKeyValidation({});
+      const event = jasmine.createSpyObj('event', ['preventDefault']);
+      spyOn(AddSshKeyValidation, 'isPublicKey').and.returnValue(false);
+      spyOn(addSshKeyValidation, 'toggleWarning');
+
+      addSshKeyValidation.submit(event);
+
+      expect(event.preventDefault).toHaveBeenCalled();
+      expect(addSshKeyValidation.toggleWarning).toHaveBeenCalledWith(true);
+    });
+  });
+
+  describe('toggleWarning', () => {
+    it('shows warningElement and hides originalSubmitElement if isVisible is true', () => {
+      const warningElement = document.createElement('div');
+      const originalSubmitElement = document.createElement('div');
+      warningElement.classList.add('hide');
+
+      const addSshKeyValidation = new AddSshKeyValidation(
+        {},
+        warningElement,
+        originalSubmitElement,
+      );
+      addSshKeyValidation.toggleWarning(true);
+
+      expect(warningElement.classList.contains('hide')).toBeFalsy();
+      expect(originalSubmitElement.classList.contains('hide')).toBeTruthy();
+    });
+
+    it('hides warningElement and shows originalSubmitElement if isVisible is false', () => {
+      const warningElement = document.createElement('div');
+      const originalSubmitElement = document.createElement('div');
+      originalSubmitElement.classList.add('hide');
+
+      const addSshKeyValidation = new AddSshKeyValidation(
+        {},
+        warningElement,
+        originalSubmitElement,
+      );
+      addSshKeyValidation.toggleWarning(false);
+
+      expect(warningElement.classList.contains('hide')).toBeTruthy();
+      expect(originalSubmitElement.classList.contains('hide')).toBeFalsy();
+    });
+  });
+
+  describe('isPublicKey', () => {
+    it('returns false if probably invalid public ssh key', () => {
+      expect(AddSshKeyValidation.isPublicKey('nope')).toBeFalsy();
+    });
+
+    it('returns true if probably valid public ssh key', () => {
+      expect(AddSshKeyValidation.isPublicKey('ssh-')).toBeTruthy();
+      expect(AddSshKeyValidation.isPublicKey('ecdsa-sha2-')).toBeTruthy();
+    });
+  });
+});