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

This commit is contained in:
Luke Bennett 2018-07-06 14:36:02 +00:00 committed by Phil Hughes
parent 3033e01924
commit 02ad729308
7 changed files with 162 additions and 3 deletions

View File

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

View File

@ -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);
}
}

View File

@ -255,3 +255,8 @@ label {
color: $theme-gray-600;
}
}
.input-lg {
max-width: 320px;
width: 100%;
}

View File

@ -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"

View File

@ -0,0 +1,5 @@
---
title: Update new SSH key page to improve key input validation
merge_request: 19997
author:
type: other

View File

@ -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

View File

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