More review comments

5 files changed, 13 insertions, 3 deletions

diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb
index eeb130d5240..b5dece38de1 100644
--- a/app/helpers/form_helper.rb
+++ b/app/helpers/form_helper.rb
@@ -3,7 +3,6 @@ module FormHelper
     return unless model.errors.any?
 
     pluralized = 'error'.pluralize(model.errors.count)
-
     headline   = "The #{type} contains the following #{pluralized}:"
 
     content_tag(:div, class: 'alert alert-danger', id: 'error_explanation') do
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 2c61717d0e0..3568e72e463 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -155,6 +155,8 @@ class ApplicationSetting < ActiveRecord::Base
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end
 
+  validates :allowed_key_types, presence: true
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     value&.each do |level|
       unless Gitlab::VisibilityLevel.options.value?(level)
diff --git a/app/views/profiles/keys/_key.html.haml b/app/views/profiles/keys/_key.html.haml
index 99b9a05fff6..103446243e5 100644
--- a/app/views/profiles/keys/_key.html.haml
+++ b/app/views/profiles/keys/_key.html.haml
@@ -3,8 +3,8 @@
     - if key.valid?
       = icon 'key', class: 'settings-list-icon hidden-xs'
     - else
-      = icon 'exclamation-triangle', class: 'settings-list-icon hidden-xs',
-        title: 'The key is disabled because it is invalid'
+      = icon 'exclamation-triangle', class: 'settings-list-icon hidden-xs has-tooltip',
+        title: key.errors.full_messages.join(', ')
 
 
   .key-list-item-info
diff --git a/doc/security/img/ssh_keys_restrictions_settings.png b/doc/security/img/ssh_keys_restrictions_settings.png
index 7b8bbb05bce..2e918fd4b3f 100644
--- a/doc/security/img/ssh_keys_restrictions_settings.png
+++ b/doc/security/img/ssh_keys_restrictions_settings.png
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index a3b6baca0a2..f921545668d 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -77,6 +77,15 @@ describe ApplicationSetting do
         expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519)
       end
 
+      it 'does not allow all key types to be disabled' do
+        described_class::SUPPORTED_KEY_TYPES.each do |type|
+          setting["#{type}_key_restriction"] = described_class::FORBIDDEN_KEY_VALUE
+        end
+
+        expect(setting).not_to be_valid
+        expect(setting.errors.messages).to have_key(:allowed_key_types)
+      end
+
       where(:type) do
         described_class::SUPPORTED_KEY_TYPES
       end