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(); + }); + }); +});