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

Handle cases when authorized_keys doesn't exist

See merge request gitlab-org/gitlab-ce!26347

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,8 +8,16 @@ 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
+    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')
@@ -19,6 +27,18 @@ describe Gitlab::AuthorizedKeys do
       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
+
   describe '#batch_add_keys' do
     let(:keys) do
       [
@@ -27,10 +47,15 @@ describe Gitlab::AuthorizedKeys do
       ]
     end
 
+    context 'authorized_keys file exists' do
       before do
         create_authorized_keys_fixture
       end
 
+      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"
@@ -51,9 +76,29 @@ describe Gitlab::AuthorizedKeys do
       end
     end
 
+    context 'authorized_keys file does not exist' do
+      before do
+        delete_authorized_keys_file
+      end
+
+      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
+    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(/./, '#')
@@ -68,13 +113,45 @@ describe Gitlab::AuthorizedKeys do
       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
+    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
+    context 'authorized_keys file exists' do
       before do
         create_authorized_keys_fixture(
           existing_content:
@@ -82,16 +159,35 @@ describe Gitlab::AuthorizedKeys do
         )
       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
 
+    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
+
   def create_authorized_keys_fixture(existing_content: 'existing content')
     FileUtils.mkdir_p(File.dirname(tmp_authorized_keys_path))
     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