From 02ad729308307a107ca930485e4b5b7dae10e5dc Mon Sep 17 00:00:00 2001
From: Luke Bennett <lbennett@gitlab.com>
Date: Fri, 6 Jul 2018 14:36:02 +0000
Subject: [PATCH] (Part 2) Resolve "Recognise when a user is trying to validate
a private SSH key"
---
.../javascripts/pages/profiles/keys/index.js | 16 +++++
.../profile/add_ssh_key_validation.js | 43 ++++++++++++
app/assets/stylesheets/framework/forms.scss | 5 ++
app/views/profiles/keys/_form.html.haml | 13 +++-
...s-trying-to-validate-a-private-ssh-key.yml | 5 ++
spec/features/profiles/keys_spec.rb | 14 ++++
.../profile/add_ssh_key_validation_spec.js | 69 +++++++++++++++++++
7 files changed, 162 insertions(+), 3 deletions(-)
create mode 100644 app/assets/javascripts/pages/profiles/keys/index.js
create mode 100644 app/assets/javascripts/profile/add_ssh_key_validation.js
create mode 100644 changelogs/unreleased/46396-recognise-when-a-user-is-trying-to-validate-a-private-ssh-key.yml
create mode 100644 spec/javascripts/profile/add_ssh_key_validation_spec.js
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 0000000000000..1cd3ee1dfdba2
--- /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 0000000000000..ab6a6c1896ca7
--- /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 282e424fc38e1..a22454c24e2cb 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 c14700794ceee..43a2d53b84d06 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 0000000000000..64bbecf340558
--- /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 bfb17a5661382..e6586fc8a0a45 100644
--- a/spec/features/profiles/keys_spec.rb
+++ b/spec/features/profiles/keys_spec.rb
@@ -30,6 +30,20 @@
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 0000000000000..c71a2885acc38
--- /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();
+ });
+ });
+});
--
GitLab