Further fixes and improvements

Signed-off-by: Rémy Coutable <remy@rymai.me>

19 files changed, 158 insertions, 77 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index f1847c06823..6cabd9eb7fd 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -63,14 +63,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       end
     end
 
-    allowed_key_types = params[:application_setting][:allowed_key_types]
-    if allowed_key_types.nil?
-      params[:application_setting][:allowed_key_types] = []
-    else
-      allowed_key_types.map! do |type|
-        type.to_sym
-      end
-    end
+    params[:application_setting][:allowed_key_types] = Array(params[:application_setting][:allowed_key_types])
 
     enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources)
 
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 37fc0e37467..d0af5583de4 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -95,13 +95,14 @@ module ApplicationSettingsHelper
 
   def allowed_key_types_checkboxes(help_block_id)
     Gitlab::SSHPublicKey::TYPES.map do |type|
-      checked = current_application_settings.allowed_key_types.include?(type.to_sym)
-      checkbox_name = 'application_setting[allowed_key_types][]'
+      checked = current_application_settings.allowed_key_types.include?(type)
+      checkbox_id = "allowed_key_types-#{type}"
 
-      label_tag(checkbox_name, class: checked ? 'active' : nil) do
-        check_box_tag(checkbox_name, type, checked,
+      label_tag(checkbox_id, class: checked ? 'active' : nil) do
+        check_box_tag('application_setting[allowed_key_types][]', type, checked,
                       autocomplete: 'off',
-                      'aria-describedby' => help_block_id) + type.upcase
+                      'aria-describedby' => help_block_id,
+                      id: checkbox_id) + type.upcase
       end
     end
   end
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index f335f9d7f85..77582df2a48 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -116,6 +116,8 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 
+  validates :allowed_key_types, presence: true
+
   validates :minimum_rsa_bits,
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
@@ -149,11 +151,9 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   validates_each :allowed_key_types do |record, attr, value|
-    unless value.nil?
-      value.each do |type|
-        unless Gitlab::SSHPublicKey::TYPES.include?(type.to_sym)
-          record.errors.add(attr, "'#{type}' is not an valid SSH key type")
-        end
+    value&.each do |type|
+      unless Gitlab::SSHPublicKey.allowed_type?(type)
+        record.errors.add(attr, "'#{type}' is not a valid SSH key type")
       end
     end
   end
@@ -206,8 +206,8 @@ class ApplicationSetting < ActiveRecord::Base
       koding_url: nil,
       max_artifacts_size: Settings.artifacts['max_size'],
       max_attachment_size: Settings.gitlab['max_attachment_size'],
-      minimum_rsa_bits: 1024,
       minimum_ecdsa_bits: 256,
+      minimum_rsa_bits: 1024,
       plantuml_enabled: false,
       plantuml_url: nil,
       recaptcha_enabled: false,
@@ -304,14 +304,11 @@ class ApplicationSetting < ActiveRecord::Base
     sidekiq_throttling_enabled
   end
 
-  def allowed_key_types
-    read_attribute(:allowed_key_types).map(&:to_sym)
-  end
-
   private
 
   def check_repository_storages
     invalid = repository_storages - Gitlab.config.repositories.storages.keys
-    errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless invalid.empty?
+    errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless
+      invalid.empty?
   end
 end
diff --git a/app/models/key.rb b/app/models/key.rb
index 74f25739f09..d8ba15da8ea 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -95,8 +95,6 @@ class Key < ActiveRecord::Base
   end
 
   def key_meets_minimum_bit_length
-    return unless key?
-
     case public_key.type
     when :ecdsa
       if public_key.size < current_application_settings.minimum_ecdsa_bits
@@ -110,10 +108,8 @@ class Key < ActiveRecord::Base
   end
 
   def key_type_is_allowed
-    return unless key?
-
-    unless current_application_settings.allowed_key_types.include?(public_key.type)
-      allowed_types = current_application_settings.allowed_key_types.to_sentence(last_word_connector: ', or ', two_words_connector: ' or ')
+    unless current_application_settings.allowed_key_types.include?(public_key.type.to_s)
+      allowed_types = current_application_settings.allowed_key_types.map(&:upcase).to_sentence(last_word_connector: ', or ', two_words_connector: ' or ')
       errors.add(:key, "type is not allowed. Must be #{allowed_types}")
     end
   end
diff --git a/app/views/layouts/_search.html.haml b/app/views/layouts/_search.html.haml
index 0e64ebd71b8..da5b20b9329 100644
--- a/app/views/layouts/_search.html.haml
+++ b/app/views/layouts/_search.html.haml
@@ -43,5 +43,5 @@
     - if @snippet || @snippets
       = hidden_field_tag :snippets, true
     = hidden_field_tag :repository_ref, @ref
-    = button_tag 'Go' if ENV['RAILS_ENV'] == 'test'
+    = button_tag 'Go' if Rails.env.test?
     .search-autocomplete-opts.hide{ :'data-autocomplete-path' => search_autocomplete_path, :'data-autocomplete-project-id' => @project.try(:id), :'data-autocomplete-project-ref' => @ref }
diff --git a/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb b/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb
index 8eefd8f8a65..327e260484e 100644
--- a/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb
+++ b/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb
@@ -9,7 +9,7 @@ class AddMinimumKeyLengthToApplicationSettings < ActiveRecord::Migration
   def up
     add_column_with_default :application_settings, :minimum_rsa_bits, :integer, default: 1024
     add_column_with_default :application_settings, :minimum_ecdsa_bits, :integer, default: 256
-    add_column_with_default :application_settings, :allowed_key_types, :text, default: %w[rsa dsa ecdsa].to_yaml
+    add_column_with_default :application_settings, :allowed_key_types, :string, default: %w[rsa dsa ecdsa].to_yaml
   end
 
   def down
diff --git a/db/schema.rb b/db/schema.rb
index 511e561e83f..d39ca120dd2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -100,7 +100,7 @@ ActiveRecord::Schema.define(version: 20170215200045) do
     t.text "after_sign_up_text_html"
     t.integer "minimum_rsa_bits", default: 1024, null: false
     t.integer "minimum_ecdsa_bits", default: 256, null: false
-    t.text "allowed_key_types", default: "---\n- rsa\n- dsa\n- ecdsa\n", null: false
+    t.string "allowed_key_types", default: "---\n- rsa\n- dsa\n- ecdsa\n", null: false
     t.boolean "housekeeping_enabled", default: true, null: false
     t.boolean "housekeeping_bitmaps_enabled", default: true, null: false
     t.integer "housekeeping_incremental_repack_period", default: 10, null: false
@@ -114,9 +114,6 @@ ActiveRecord::Schema.define(version: 20170215200045) do
     t.boolean "plantuml_enabled"
     t.integer "max_pages_size", default: 100, null: false
     t.integer "terminal_max_session_time", default: 0, null: false
-    t.integer "minimum_rsa_bits", default: 1024
-    t.integer "minimum_ecdsa_bits", default: 256
-    t.text "allowed_key_types"
   end
 
   create_table "audit_events", force: :cascade do |t|
diff --git a/doc/api/settings.md b/doc/api/settings.md
index 791b4dd729d..8bf538a6370 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -91,7 +91,7 @@ PUT /application/settings
 | `terminal_max_session_time` | integer | no | Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time. |
 | `minimum_rsa_bits` | integer | no | The minimum allowed bit length of an uploaded RSA key. Default is `1024`.
 | `minimum_ecdsa_bits` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `256`.
-| `minimum_ecdsa_bits` | array of strings | no | Array of SSH key types accepted by the application. Allowed values are: `rsa`, `dsa`, and `ecdsa`. Default is `["rsa", "dsa", "ecdsa"]`.
+| `allowed_key_types` | array of strings | no | Array of SSH key types accepted by the application. Allowed values are: `rsa`, `dsa`, and `ecdsa`. Default is `["rsa", "dsa", "ecdsa"]`.
 
 ```bash
 curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/application/settings?signup_enabled=false&default_project_visibility=1
diff --git a/doc/security/README.md b/doc/security/README.md
index 38706e48ec5..1f54948d113 100644
--- a/doc/security/README.md
+++ b/doc/security/README.md
@@ -1,6 +1,7 @@
 # Security
 
 - [Password length limits](password_length_limits.md)
+- [Restrict allowed SSH key technologies and minimum length](ssh_keys_restrictions.md)
 - [Rack attack](rack_attack.md)
 - [Webhooks and insecure internal web services](webhooks.md)
 - [Information exclusivity](information_exclusivity.md)
diff --git a/doc/security/img/ssh_keys_restrictions_settings.png b/doc/security/img/ssh_keys_restrictions_settings.png
new file mode 100644
index 00000000000..fef65f85011
--- /dev/null
+++ b/doc/security/img/ssh_keys_restrictions_settings.png
diff --git a/doc/security/ssh_keys_restrictions.md b/doc/security/ssh_keys_restrictions.md
new file mode 100644
index 00000000000..32ca7dacab3
--- /dev/null
+++ b/doc/security/ssh_keys_restrictions.md
@@ -0,0 +1,18 @@
+# Restrict allowed SSH key technologies and minimum length
+
+`ssh-keygen` allows users to create RSA keys with as few as 768 bits, which
+falls well below recommendations from certain standards groups (such as the US
+NIST). Some organizations deploying Gitlab will need to enforce minimum key
+strength, either to satisfy internal security policy or for regulatory
+compliance.
+
+Similarly, certain standards groups recommend using RSA or ECDSA over the older
+DSA and administrators may need to limit the allowed SSH key algorithms.
+
+GitLab allows you to restrict the allowed SSH key technology as well as specify
+the minimum key length for each technology.
+
+In the Admin area under **Settings** (`/admin/application_settings`), look for
+the "Visibility and Access Controls" area:
+
+![SSH keys restriction admin settings](img/ssh_keys_restrictions_settings.png)
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 747ceb4e3e0..5a72960b0f3 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -109,6 +109,10 @@ module API
         requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run."
       end
       optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
+      optional :minimum_rsa_bits, type: Integer, desc: 'The minimum allowed bit length of an uploaded RSA key.'
+      optional :minimum_ecdsa_bits, type: Integer, desc: 'The minimum allowed curve size (in bits) of an uploaded ECDSA key.'
+      optional :allowed_key_types, type: Array[String], values: Gitlab::SSHPublicKey::TYPES, desc: 'The SSH key types accepted by the application (`rsa`, `dsa`, or `ecdsa`).'
+
       at_least_one_of :default_branch_protection, :default_project_visibility, :default_snippet_visibility,
                       :default_group_visibility, :restricted_visibility_levels, :import_sources,
                       :enabled_git_access_protocol, :gravatar_enabled, :default_projects_limit,
diff --git a/lib/gitlab/ssh_public_key.rb b/lib/gitlab/ssh_public_key.rb
index e2af4597c82..2261ec57d9c 100644
--- a/lib/gitlab/ssh_public_key.rb
+++ b/lib/gitlab/ssh_public_key.rb
@@ -4,9 +4,11 @@ module Gitlab
   class SSHPublicKey
     include Gitlab::Popen
 
-    UnsupportedSSHPublicKeyTypeError = Class.new(ArgumentError)
+    TYPES = %w[rsa dsa ecdsa].freeze
 
-    TYPES = %i[rsa dsa ecdsa].freeze
+    def self.allowed_type?(type)
+      TYPES.include?(type.to_s)
+    end
 
     def initialize(key_text)
       @key_text = key_text
@@ -17,7 +19,9 @@ module Gitlab
     end
 
     def type
-      @type ||=
+      return @type if defined?(@type)
+
+      @type =
         case key
         when OpenSSL::PKey::EC
           :ecdsa
@@ -25,13 +29,13 @@ module Gitlab
           :rsa
         when OpenSSL::PKey::DSA
           :dsa
-        else
-          raise UnsupportedSSHPublicKeyTypeError, "#{key.class} is not supported"
         end
     end
 
     def size
-      @size ||=
+      return @size if defined?(@size)
+
+      @size =
         case type
         when :ecdsa
           key.public_key.to_bn.num_bits / 2
@@ -39,8 +43,6 @@ module Gitlab
           key.n.num_bits
         when :dsa
           1024
-        else
-          raise UnsupportedSSHPublicKeyTypeError, "#{key.class} is not supported"
         end
     end
 
@@ -87,7 +89,7 @@ module Gitlab
       # OpenSSH 6.8 introduces a new default output format for fingerprints.
       # Check the version and decide which command to use.
 
-      version_output, version_status = popen(%w(ssh -V))
+      version_output, version_status = popen(%w[ssh -V])
       return false unless version_status.zero?
 
       version_matches = version_output.match(/OpenSSH_(?<major>\d+)\.(?<minor>\d+)/)
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index de42ab81fac..97592aa70b0 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -43,6 +43,24 @@ feature 'Admin updates settings', feature: true do
     expect(find('#service_push_channel').value).to eq '#test_channel'
   end
 
+  scenario 'Change Keys settings' do
+    uncheck 'RSA'
+    uncheck 'DSA'
+    fill_in 'Minimum ECDSA key length', with: '384'
+    click_on 'Save'
+
+    expect(page).to have_content 'Application settings saved successfully'
+
+    expect(find_field('RSA', checked: false)).not_to be_checked
+    expect(find_field('DSA', checked: false)).not_to be_checked
+    expect(find_field('Minimum ECDSA key length').value).to eq '384'
+
+    uncheck 'ECDSA'
+    click_on 'Save'
+
+    expect(page).to have_content "Allowed key types can't be blank"
+  end
+
   def check_all_events
     page.check('Active')
     page.check('Push')
diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb
index eb1050d21c6..8cc5c28975b 100644
--- a/spec/features/profiles/keys_spec.rb
+++ b/spec/features/profiles/keys_spec.rb
@@ -28,6 +28,22 @@ feature 'Profile > SSH Keys', feature: true do
       expect(page).to have_content("Title: #{attrs[:title]}")
       expect(page).to have_content(attrs[:key])
     end
+
+    context 'when only DSA and ECDSA keys are allowed' do
+      before do
+        stub_application_setting(allowed_key_types: %w[dsa ecdsa])
+      end
+
+      scenario 'shows a validation error' do
+        attrs = attributes_for(:key)
+
+        fill_in('Key', with: attrs[:key])
+        fill_in('Title', with: attrs[:title])
+        click_button('Add key')
+
+        expect(page).to have_content('Key type is not allowed. Must be DSA or ECDSA')
+      end
+    end
   end
 
   scenario 'User sees their keys' do
diff --git a/spec/lib/gitlab/ssh_public_key_spec.rb b/spec/lib/gitlab/ssh_public_key_spec.rb
index ae50f785698..60df8d37b4c 100644
--- a/spec/lib/gitlab/ssh_public_key_spec.rb
+++ b/spec/lib/gitlab/ssh_public_key_spec.rb
@@ -1,20 +1,60 @@
 require 'spec_helper'
 
 describe Gitlab::SSHPublicKey, lib: true do
+  let(:key) { attributes_for(:key)[:key] }
   let(:public_key) { described_class.new(key) }
-  let(:key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDPizF8D6ywvnsLKmGH8LjUku9L5YGbnM3RkSQgNxzem6YBCYQ7HHSipqGTYSFBGnNzHm7Ndj0BrMH8ZTwn+X0F3Q+6gUQe/v37OMHhBOazdxU3RDZzrlQs8qqkQr9mqJJcvuCdDI03hoVFEkZg6TzwIv0Sk7dBP4FOG3j83oZ8rQ== dummy@gitlab.com' }
 
-  describe 'unknown key type' do
+  describe '.allowed_type?' do
     it 'determines the key type' do
-      ssh_key = described_class.new('foo')
+      expect(described_class.allowed_type?('foo')).to be(false)
+    end
+  end
 
-      expect { ssh_key.type }.to raise Gitlab::SSHPublicKey::UnsupportedSSHPublicKeyTypeError
+  describe '#valid?' do
+    context 'with a valid SSH key' do
+      it 'returns true' do
+        expect(public_key).to be_valid
+      end
+    end
+
+    context 'with an invalid SSH key' do
+      let(:key) { 'this is not a key' }
+
+      it 'returns false' do
+        expect(public_key).not_to be_valid
+      end
     end
   end
 
   describe '#type' do
-    it 'determines the key type' do
-      expect(public_key.type).to eq(:rsa)
+    context 'with a DSA key' do
+      let(:key) { attributes_for(:dsa_key)[:key] }
+
+      it 'determines the key type' do
+        expect(public_key.type).to eq(:dsa)
+      end
+    end
+
+    context 'with a ECDSA key' do
+      let(:key) { attributes_for(:ecdsa_key)[:key] }
+
+      it 'determines the key type' do
+        expect(public_key.type).to eq(:ecdsa)
+      end
+    end
+
+    context 'with a RSA key' do
+      it 'determines the key type' do
+        expect(public_key.type).to eq(:rsa)
+      end
+    end
+
+    context 'with an invalid SSH key' do
+      let(:key) { 'this is not a key' }
+
+      it 'determines the key type' do
+        expect(public_key.type).to be_nil
+      end
     end
   end
 
@@ -22,26 +62,17 @@ describe Gitlab::SSHPublicKey, lib: true do
     it 'determines the key length in bits' do
       expect(public_key.size).to eq(1024)
     end
-  end
-
-  describe '#valid?' do
-    context 'with a valid SSH key' do
-      it 'returns true' do
-        expect(public_key.valid?).to eq(true)
-      end
-    end
 
     context 'with an invalid SSH key' do
       let(:key) { 'this is not a key' }
 
-      it 'returns false' do
-        expect(public_key.valid?).to eq(false)
+      it 'determines the key type' do
+        expect(public_key.size).to be_nil
       end
     end
   end
 
   describe '#fingerprint' do
-    let(:key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=' }
     let(:fingerprint) { '3f:a2:ee:de:b5:de:53:c3:aa:2f:9c:45:24:4c:47:7b' }
 
     it "generates the key's fingerprint" do
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index bbae6ce39df..55712368cdd 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -195,7 +195,7 @@ describe ApplicationSetting, models: true do
 
     it 'get value as array of symbols' do
       setting.allowed_key_types = ['rsa']
-      expect(setting.allowed_key_types).to eq([:rsa])
+      expect(setting.allowed_key_types).to eq(['rsa'])
     end
   end
 end
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 64a643738d6..f461b3ba49d 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -19,8 +19,9 @@ describe Key, models: true do
 
     it { is_expected.to validate_presence_of(:key) }
     it { is_expected.to validate_length_of(:key).is_at_most(5000) }
-    it { is_expected.to allow_value('ssh-foo').for(:key) }
-    it { is_expected.to allow_value('ecdsa-foo').for(:key) }
+    it { is_expected.to allow_value(attributes_for(:dsa_key)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:ecdsa_key)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:key)[:key]).for(:key) }
     it { is_expected.not_to allow_value('foo-bar').for(:key) }
   end
 
@@ -136,25 +137,33 @@ describe Key, models: true do
   end
 
   context 'validate the key type is allowed' do
-    it 'accepts a key if type is allowed' do
+    it 'accepts RSA, ECDSA, and DSA keys by default' do
       expect(build(:key)).to be_valid
+      expect(build(:dsa_key)).to be_valid
+      expect(build(:ecdsa_key)).to be_valid
     end
 
-    it 'rejects RSA key if RSA is not an allowed type' do
-      stub_application_setting(allowed_key_types: [:dsa])
+    it 'rejects RSA and ECDSA key if DSA is the only allowed type' do
+      stub_application_setting(allowed_key_types: ['dsa'])
 
       expect(build(:key)).not_to be_valid
+      expect(build(:dsa_key)).to be_valid
+      expect(build(:ecdsa_key)).not_to be_valid
     end
 
-    it 'rejects DSA key if DSA is not an allowed type' do
-      stub_application_setting(allowed_key_types: [:ecdsa])
+    it 'rejects RSA and DSA key if ECDSA is the only allowed type' do
+      stub_application_setting(allowed_key_types: ['ecdsa'])
 
+      expect(build(:key)).not_to be_valid
       expect(build(:dsa_key)).not_to be_valid
+      expect(build(:ecdsa_key)).to be_valid
     end
 
-    it 'rejects ECDSA key if ECDSA is not an allowed type' do
-      stub_application_setting(allowed_key_types: [:rsa])
+    it 'rejects DSA and ECDSA key if RSA is the only allowed type' do
+      stub_application_setting(allowed_key_types: ['rsa'])
 
+      expect(build(:key)).to be_valid
+      expect(build(:dsa_key)).not_to be_valid
       expect(build(:ecdsa_key)).not_to be_valid
     end
   end
@@ -174,9 +183,7 @@ describe Key, models: true do
   end
 
   describe '#key=' do
-    let(:valid_key) do
-      "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com"
-    end
+    let(:valid_key) { attributes_for(:key)[:key] }
 
     it 'strips white spaces' do
       expect(described_class.new(key: " #{valid_key} ").key).to eq(valid_key)
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 1a8d2c2ef4e..f47e6250d50 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -20,7 +20,7 @@ describe API::Settings, 'Settings', api: true  do
       expect(json_response['plantuml_url']).to be_nil
       expect(json_response['minimum_rsa_bits']).to eq(1024)
       expect(json_response['minimum_ecdsa_bits']).to eq(256)
-      expect(json_response['allowed_key_types']).to eq(['rsa', 'dsa', 'ecdsa'])
+      expect(json_response['allowed_key_types']).to eq(%w[rsa dsa ecdsa])
     end
   end