Merge branch '10514-fix-non-existing-authorized-keys-file-ee' into 'master'

Handle cases when authorized_keys doesn't exist

Closes #10514

See merge request gitlab-org/gitlab-ee!10304

ee/spec/migrations/update_authorized_keys_file_spec.rb

@@ -16,8 +16,6 @@ describe UpdateAuthorizedKeysFile, :migration do
       end
 
       it 'sets authorized_keys_enabled to true' do
-        FileUtils.touch(authorized_keys_file)
-
         migration.up
 
         expect(described_class::ApplicationSetting.last.authorized_keys_enabled).to be_truthy

lib/gitlab/authorized_keys.rb

@@ -68,6 +68,8 @@ module Gitlab
       end
 
       true
+    rescue Errno::ENOENT
+      false
     end
 
     # Clear the authorized_keys file
@@ -96,6 +98,8 @@ module Gitlab
           end
         end
       end
+    rescue Errno::ENOENT
+      []
     end
 
     private

spec/lib/gitlab/authorized_keys_spec.rb

@@ -8,14 +8,34 @@ describe Gitlab::AuthorizedKeys do
   subject { described_class.new(logger) }
 
   describe '#add_key' do
-    it "adds a line at the end of the file and strips trailing garbage" do
-      create_authorized_keys_fixture
-      auth_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaDAxx2E"
-
-      expect(logger).to receive(:info).with('Adding key (key-741): ssh-rsa AAAAB3NzaDAxx2E')
-      expect(subject.add_key('key-741', 'ssh-rsa AAAAB3NzaDAxx2E trailing garbage'))
-        .to be_truthy
-      expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line}\n")
+    context 'authorized_keys file exists' do
+      before do
+        create_authorized_keys_fixture
+      end
+
+      after do
+        delete_authorized_keys_file
+      end
+
+      it "adds a line at the end of the file and strips trailing garbage" do
+        auth_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaDAxx2E"
+
+        expect(logger).to receive(:info).with('Adding key (key-741): ssh-rsa AAAAB3NzaDAxx2E')
+        expect(subject.add_key('key-741', 'ssh-rsa AAAAB3NzaDAxx2E trailing garbage'))
+          .to be_truthy
+        expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line}\n")
+      end
+    end
+
+    context 'authorized_keys file does not exist' do
+      before do
+        delete_authorized_keys_file
+      end
+
+      it 'creates the file' do
+        expect(subject.add_key('key-741', 'ssh-rsa AAAAB3NzaDAxx2E')).to be_truthy
+        expect(File.exist?(tmp_authorized_keys_path)).to be_truthy
+      end
     end
   end
 
@@ -27,63 +47,135 @@ describe Gitlab::AuthorizedKeys do
       ]
     end
 
-    before do
-      create_authorized_keys_fixture
-    end
+    context 'authorized_keys file exists' do
+      before do
+        create_authorized_keys_fixture
+      end
 
-    it "adds lines at the end of the file" do
-      auth_line1 = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-12\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-dsa ASDFASGADG"
-      auth_line2 = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-123\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa GFDGDFSGSDFG"
+      after do
+        delete_authorized_keys_file
+      end
+
+      it "adds lines at the end of the file" do
+        auth_line1 = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-12\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-dsa ASDFASGADG"
+        auth_line2 = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-123\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa GFDGDFSGSDFG"
+
+        expect(logger).to receive(:info).with('Adding key (key-12): ssh-dsa ASDFASGADG')
+        expect(logger).to receive(:info).with('Adding key (key-123): ssh-rsa GFDGDFSGSDFG')
+        expect(subject.batch_add_keys(keys)).to be_truthy
+        expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line1}\n#{auth_line2}\n")
+      end
+
+      context "invalid key" do
+        let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] }
 
-      expect(logger).to receive(:info).with('Adding key (key-12): ssh-dsa ASDFASGADG')
-      expect(logger).to receive(:info).with('Adding key (key-123): ssh-rsa GFDGDFSGSDFG')
-      expect(subject.batch_add_keys(keys)).to be_truthy
-      expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line1}\n#{auth_line2}\n")
+        it "doesn't add keys" do
+          expect(subject.batch_add_keys(keys)).to be_falsey
+          expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n")
+        end
+      end
     end
 
-    context "invalid key" do
-      let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] }
+    context 'authorized_keys file does not exist' do
+      before do
+        delete_authorized_keys_file
+      end
 
-      it "doesn't add keys" do
-        expect(subject.batch_add_keys(keys)).to be_falsey
-        expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n")
+      it 'creates the file' do
+        expect(subject.batch_add_keys(keys)).to be_truthy
+        expect(File.exist?(tmp_authorized_keys_path)).to be_truthy
       end
     end
   end
 
   describe '#rm_key' do
-    it "removes the right line" do
-      create_authorized_keys_fixture
-      other_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E"
-      delete_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E"
-      erased_line = delete_line.gsub(/./, '#')
-      File.open(tmp_authorized_keys_path, 'a') do |auth_file|
-        auth_file.puts delete_line
-        auth_file.puts other_line
-      end
-
-      expect(logger).to receive(:info).with('Removing key (key-741)')
-      expect(subject.rm_key('key-741')).to be_truthy
-      expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{erased_line}\n#{other_line}\n")
+    context 'authorized_keys file exists' do
+      before do
+        create_authorized_keys_fixture
+      end
+
+      after do
+        delete_authorized_keys_file
+      end
+
+      it "removes the right line" do
+        other_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E"
+        delete_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E"
+        erased_line = delete_line.gsub(/./, '#')
+        File.open(tmp_authorized_keys_path, 'a') do |auth_file|
+          auth_file.puts delete_line
+          auth_file.puts other_line
+        end
+
+        expect(logger).to receive(:info).with('Removing key (key-741)')
+        expect(subject.rm_key('key-741')).to be_truthy
+        expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{erased_line}\n#{other_line}\n")
+      end
+    end
+
+    context 'authorized_keys file does not exist' do
+      before do
+        delete_authorized_keys_file
+      end
+
+      it 'returns false' do
+        expect(subject.rm_key('key-741')).to be_falsey
+      end
     end
   end
 
   describe '#clear' do
-    it "should return true" do
-      expect(subject.clear).to be_truthy
+    context 'authorized_keys file exists' do
+      before do
+        create_authorized_keys_fixture
+      end
+
+      after do
+        delete_authorized_keys_file
+      end
+
+      it "returns true" do
+        expect(subject.clear).to be_truthy
+      end
+    end
+
+    context 'authorized_keys file does not exist' do
+      before do
+        delete_authorized_keys_file
+      end
+
+      it "still returns true" do
+        expect(subject.clear).to be_truthy
+      end
     end
   end
 
   describe '#list_key_ids' do
-    before do
-      create_authorized_keys_fixture(
-        existing_content:
-          "key-1\tssh-dsa AAA\nkey-2\tssh-rsa BBB\nkey-3\tssh-rsa CCC\nkey-9000\tssh-rsa DDD\n"
-      )
+    context 'authorized_keys file exists' do
+      before do
+        create_authorized_keys_fixture(
+          existing_content:
+            "key-1\tssh-dsa AAA\nkey-2\tssh-rsa BBB\nkey-3\tssh-rsa CCC\nkey-9000\tssh-rsa DDD\n"
+        )
+      end
+
+      after do
+        delete_authorized_keys_file
+      end
+
+      it 'returns array of key IDs' do
+        expect(subject.list_key_ids).to eq([1, 2, 3, 9000])
+      end
     end
 
-    it 'returns array of key IDs' do
-      expect(subject.list_key_ids).to eq([1, 2, 3, 9000])
+    context 'authorized_keys file does not exist' do
+      before do
+        delete_authorized_keys_file
+      end
+
+      it 'returns an empty array' do
+        expect(subject.list_key_ids).to be_empty
+      end
     end
   end
 
@@ -92,6 +184,10 @@ describe Gitlab::AuthorizedKeys do
     File.open(tmp_authorized_keys_path, 'w') { |file| file.puts(existing_content) }
   end
 
+  def delete_authorized_keys_file
+    File.delete(tmp_authorized_keys_path) if File.exist?(tmp_authorized_keys_path)
+  end
+
   def tmp_authorized_keys_path
     Gitlab.config.gitlab_shell.authorized_keys_file
   end