Merge branch 'improve-ssh-keys-page' into 'master'

Improve 'Add an SSH key' page

See merge request gitlab-org/gitlab!77403

app/helpers/ssh_keys_helper.rb

@@ -18,4 +18,14 @@ module SshKeysHelper
         container: 'body'
     }
   end
+
+  def ssh_key_allowed_algorithms
+    allowed_algorithms = Gitlab::CurrentSettings.allowed_key_types.flat_map do |ssh_key_type_name|
+      Gitlab::SSHPublicKey.supported_algorithms_for_name(ssh_key_type_name)
+    end
+
+    quoted_allowed_algorithms = allowed_algorithms.map { |name| "'#{name}'" }
+
+    Gitlab::Utils.to_exclusive_sentence(quoted_allowed_algorithms)
+  end
 end

app/views/profiles/gpg_keys/index.html.haml

@@ -12,7 +12,7 @@
       = _('Add a GPG key')
     %p.profile-settings-content
       - help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/project/repository/gpg_signed_commits/index.md') }
-      = _('Before you can add a GPG key you need to %{help_link_start}Generate it.%{help_link_end}'.html_safe) % {help_link_start: help_link_start, help_link_end: '</a>'.html_safe }
+      = _('Add a GPG key for secure access to GitLab. %{help_link_start}Learn more.%{help_link_end}').html_safe % {help_link_start: help_link_start, help_link_end: '</a>'.html_safe }
     = render 'form'
     %hr
     %h5

app/views/profiles/keys/_form.html.haml

@@ -5,8 +5,8 @@
 
     .form-group
       = f.label :key, s_('Profiles|Key'), class: 'label-bold'
-      %p= _("Paste your public SSH key, which is usually contained in the file '~/.ssh/id_ed25519.pub' or '~/.ssh/id_rsa.pub' and begins with 'ssh-ed25519' or 'ssh-rsa'. Do not paste your private SSH key, as that can compromise your identity.")
-      = f.text_area :key, class: "form-control gl-form-input js-add-ssh-key-validation-input qa-key-public-key-field", rows: 8, required: true, placeholder: s_('Profiles|Typically starts with "ssh-ed25519 …" or "ssh-rsa …"')
+      = f.text_area :key, class: "form-control gl-form-input js-add-ssh-key-validation-input qa-key-public-key-field", rows: 8, required: true
+      %p.form-text.text-muted= s_('Profiles|Begins with %{ssh_key_algorithms}.') % { ssh_key_algorithms: ssh_key_allowed_algorithms }
     .form-row
       .col.form-group
         = f.label :title, _('Title'), class: 'label-bold'

app/views/profiles/keys/index.html.haml

@@ -11,11 +11,8 @@
     %h5.gl-mt-0
       = _('Add an SSH key')
     %p.profile-settings-content
-      - generate_link_url = help_page_path("ssh/index", anchor: 'generate-an-ssh-key-pair')
-      - existing_link_url = help_page_path("ssh/index", anchor: 'see-if-you-have-an-existing-ssh-key-pair')
-      - generate_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: generate_link_url }
-      - existing_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: existing_link_url }
-      = _('To add an SSH key you need to %{generate_link_start}generate one%{link_end} or use an %{existing_link_start}existing key%{link_end}.').html_safe % { generate_link_start: generate_link_start, existing_link_start: existing_link_start, link_end: '</a>'.html_safe }
+      - help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('ssh/index.md') }
+      = _('Add an SSH key for secure access to GitLab. %{help_link_start}Learn more.%{help_link_end}').html_safe % {help_link_start: help_link_start, help_link_end: '</a>'.html_safe }
     = render 'form'
     %hr
     %h5

lib/gitlab/ssh_public_key.rb

@@ -2,13 +2,15 @@
 
 module Gitlab
   class SSHPublicKey
-    Technology = Struct.new(:name, :key_class, :supported_sizes)
+    Technology = Struct.new(:name, :key_class, :supported_sizes, :supported_algorithms)
 
+    # See https://man.openbsd.org/sshd#AUTHORIZED_KEYS_FILE_FORMAT for the list of
+    # supported algorithms.
     TECHNOLOGIES = [
-      Technology.new(:rsa, OpenSSL::PKey::RSA, [1024, 2048, 3072, 4096]),
-      Technology.new(:dsa, OpenSSL::PKey::DSA, [1024, 2048, 3072]),
-      Technology.new(:ecdsa, OpenSSL::PKey::EC, [256, 384, 521]),
-      Technology.new(:ed25519, Net::SSH::Authentication::ED25519::PubKey, [256])
+      Technology.new(:rsa, OpenSSL::PKey::RSA, [1024, 2048, 3072, 4096], %w(ssh-rsa)),
+      Technology.new(:dsa, OpenSSL::PKey::DSA, [1024, 2048, 3072], %w(ssh-dss)),
+      Technology.new(:ecdsa, OpenSSL::PKey::EC, [256, 384, 521], %w(ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521)),
+      Technology.new(:ed25519, Net::SSH::Authentication::ED25519::PubKey, [256], %w(ssh-ed25519))
     ].freeze
 
     def self.technology(name)
@@ -24,7 +26,15 @@ module Gitlab
     end
 
     def self.supported_sizes(name)
-      technology(name)&.supported_sizes
+      technology(name).supported_sizes
+    end
+
+    def self.supported_algorithms
+      TECHNOLOGIES.flat_map { |tech| tech.supported_algorithms }
+    end
+
+    def self.supported_algorithms_for_name(name)
+      technology(name).supported_algorithms
     end
 
     def self.sanitize(key_content)

locale/gitlab.pot

@@ -2009,6 +2009,9 @@ msgstr ""
 msgid "Add a GPG key"
 msgstr ""
 
+msgid "Add a GPG key for secure access to GitLab. %{help_link_start}Learn more.%{help_link_end}"
+msgstr ""
+
 msgid "Add a Jaeger URL to replace this page with a link to your Jaeger server. You first need to %{link_start_tag}install Jaeger%{link_end_tag}."
 msgstr ""
 
@@ -2075,6 +2078,9 @@ msgstr ""
 msgid "Add an SSH key"
 msgstr ""
 
+msgid "Add an SSH key for secure access to GitLab. %{help_link_start}Learn more.%{help_link_end}"
+msgstr ""
+
 msgid "Add an existing issue"
 msgstr ""
 
@@ -25714,9 +25720,6 @@ msgstr ""
 msgid "Paste this DSN into your Sentry SDK"
 msgstr ""
 
-msgid "Paste your public SSH key, which is usually contained in the file '~/.ssh/id_ed25519.pub' or '~/.ssh/id_rsa.pub' and begins with 'ssh-ed25519' or 'ssh-rsa'. Do not paste your private SSH key, as that can compromise your identity."
-msgstr ""
-
 msgid "Patch to apply"
 msgstr ""
 
@@ -27121,6 +27124,9 @@ msgstr ""
 msgid "Profiles|Avatar will be removed. Are you sure?"
 msgstr ""
 
+msgid "Profiles|Begins with %{ssh_key_algorithms}."
+msgstr ""
+
 msgid "Profiles|Bio"
 msgstr ""
 
@@ -27370,9 +27376,6 @@ msgstr ""
 msgid "Profiles|Type your %{confirmationValue} to confirm:"
 msgstr ""
 
-msgid "Profiles|Typically starts with \"ssh-ed25519 …\" or \"ssh-rsa …\""
-msgstr ""
-
 msgid "Profiles|Update profile settings"
 msgstr ""
 
@@ -37089,9 +37092,6 @@ msgstr ""
 msgid "To add a custom suffix, set up a Service Desk email address. %{linkStart}Learn more.%{linkEnd}"
 msgstr ""
 
-msgid "To add an SSH key you need to %{generate_link_start}generate one%{link_end} or use an %{existing_link_start}existing key%{link_end}."
-msgstr ""
-
 msgid "To add the entry manually, provide the following details to the application on your phone."
 msgstr ""
 

spec/helpers/ssh_keys_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SshKeysHelper do
+  describe '#ssh_key_allowed_algorithms' do
+    it 'returns string with the names of allowed algorithms that are quoted and joined by commas' do
+      allowed_algorithms = Gitlab::CurrentSettings.allowed_key_types.flat_map do |ssh_key_type_name|
+        Gitlab::SSHPublicKey.supported_algorithms_for_name(ssh_key_type_name)
+      end
+
+      quoted_allowed_algorithms = allowed_algorithms.map { |name| "'#{name}'" }
+
+      expected_string = Gitlab::Utils.to_exclusive_sentence(quoted_allowed_algorithms)
+
+      expect(ssh_key_allowed_algorithms).to eq(expected_string)
+    end
+
+    it 'returns only allowed algorithms' do
+      expect(ssh_key_allowed_algorithms).to match('ed25519')
+      stub_application_setting(ed25519_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE)
+      expect(ssh_key_allowed_algorithms).not_to match('ed25519')
+    end
+  end
+end

spec/lib/gitlab/ssh_public_key_spec.rb

@@ -39,14 +39,43 @@ RSpec.describe Gitlab::SSHPublicKey, lib: true do
       ]
     end
 
-    subject { described_class.supported_sizes(name) }
-
     with_them do
       it { expect(described_class.supported_sizes(name)).to eq(sizes) }
       it { expect(described_class.supported_sizes(name.to_s)).to eq(sizes) }
     end
   end
 
+  describe '.supported_algorithms' do
+    it 'returns all supported algorithms' do
+      expect(described_class.supported_algorithms).to eq(
+        %w(
+        ssh-rsa
+        ssh-dss
+        ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521
+        ssh-ed25519
+        )
+      )
+    end
+  end
+
+  describe '.supported_algorithms_for_name' do
+    where(:name, :algorithms) do
+      [
+        [:rsa, %w(ssh-rsa)],
+        [:dsa, %w(ssh-dss)],
+        [:ecdsa, %w(ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521)],
+        [:ed25519, %w(ssh-ed25519)]
+      ]
+    end
+
+    with_them do
+      it "returns all supported algorithms for #{params[:name]}" do
+        expect(described_class.supported_algorithms_for_name(name)).to eq(algorithms)
+        expect(described_class.supported_algorithms_for_name(name.to_s)).to eq(algorithms)
+      end
+    end
+  end
+
   describe '.sanitize(key_content)' do
     let(:content) { build(:key).key }
 

spec/views/profiles/keys/_form.html.haml_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'profiles/keys/_form.html.haml' do
+  include SshKeysHelper
+
   let_it_be(:key) { Key.new }
 
   let(:page) { Capybara::Node::Simple.new(rendered) }
@@ -23,8 +25,8 @@ RSpec.describe 'profiles/keys/_form.html.haml' do
     end
 
     it 'has the key field', :aggregate_failures do
-      expect(rendered).to have_field('Key', type: 'textarea', placeholder: 'Typically starts with "ssh-ed25519 …" or "ssh-rsa …"')
-      expect(rendered).to have_text("Paste your public SSH key, which is usually contained in the file '~/.ssh/id_ed25519.pub' or '~/.ssh/id_rsa.pub' and begins with 'ssh-ed25519' or 'ssh-rsa'. Do not paste your private SSH key, as that can compromise your identity.")
+      expect(rendered).to have_field('Key', type: 'textarea')
+      expect(rendered).to have_text(s_('Profiles|Begins with %{ssh_key_algorithms}.') % { ssh_key_algorithms: ssh_key_allowed_algorithms })
     end
 
     it 'has the title field', :aggregate_failures do