Integrate Gitlab::Keys with Gitlab::Shell

In this commit, some methods that aren't being used
are removed from `Gitlab::Shell`. They are the ff:
- `#remove_keys_not_found_in_db`
- `#batch_read_key_ids`
- `#list_key_ids`

The corresponding methods in `Gitlab::Keys` have been
removed as well.

2 files changed, 487 insertions, 195 deletions

diff --git a/spec/lib/gitlab/authorized_keys_spec.rb b/spec/lib/gitlab/authorized_keys_spec.rb
new file mode 100644
index 00000000000..b0fbe959ff8
--- /dev/null
+++ b/spec/lib/gitlab/authorized_keys_spec.rb
@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::AuthorizedKeys do
+  let(:logger) { double('logger').as_null_object }
+
+  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")
+    end
+  end
+
+  describe '#batch_add_keys' do
+    let(:keys) do
+      [
+        double(shell_id: 'key-12', key: 'ssh-dsa ASDFASGADG trailing garbage'),
+        double(shell_id: 'key-123', key: 'ssh-rsa GFDGDFSGSDFG')
+      ]
+    end
+
+    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"
+
+      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")] }
+
+      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
+
+  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")
+    end
+  end
+
+  describe '#clear' do
+    it "should return true" do
+      expect(subject.clear).to be_truthy
+    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"
+      )
+    end
+
+    it 'returns array of key IDs' do
+      expect(subject.list_key_ids).to eq([1, 2, 3, 9000])
+    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 tmp_authorized_keys_path
+    Gitlab.config.gitlab_shell.authorized_keys_file
+  end
+end
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index d6aadf0f7de..e2f09de2808 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -8,6 +8,7 @@ describe Gitlab::Shell do
   let(:gitlab_shell) { described_class.new }
   let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } }
   let(:timeout) { Gitlab.config.gitlab_shell.git_timeout }
+  let(:gitlab_authorized_keys) { double }
 
   before do
     allow(Project).to receive(:find).and_return(project)
@@ -49,13 +50,38 @@ describe Gitlab::Shell do
 
   describe '#add_key' do
     context 'when authorized_keys_enabled is true' do
-      it 'removes trailing garbage' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
-          [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar']
-        )
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
+
+        it 'calls #gitlab_shell_fast_execute with add-key command' do
+          expect(gitlab_shell)
+            .to receive(:gitlab_shell_fast_execute)
+            .with([
+              :gitlab_shell_keys_path,
+              'add-key',
+              'key-123',
+              'ssh-rsa foobar'
+            ])
+
+          gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+        end
+      end
+
+      context 'authorized_keys_file set' do
+        it 'calls Gitlab::AuthorizedKeys#add_key with id and key' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
 
-        gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+          expect(gitlab_authorized_keys)
+            .to receive(:add_key)
+            .with('key-123', 'ssh-rsa foobar')
+
+          gitlab_shell.add_key('key-123', 'ssh-rsa foobar')
+        end
       end
     end
 
@@ -64,10 +90,24 @@ describe Gitlab::Shell do
         stub_application_setting(authorized_keys_enabled: false)
       end
 
-      it 'does nothing' do
-        expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute)
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+        end
+
+        it 'does nothing' do
+          expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute)
+
+          gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+        end
+      end
+
+      context 'authorized_keys_file set' do
+        it 'does nothing' do
+          expect(Gitlab::AuthorizedKeys).not_to receive(:new)
 
-        gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+          gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+        end
       end
     end
 
@@ -76,24 +116,89 @@ describe Gitlab::Shell do
         stub_application_setting(authorized_keys_enabled: nil)
       end
 
-      it 'removes trailing garbage' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
-          [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar']
-        )
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
+
+        it 'calls #gitlab_shell_fast_execute with add-key command' do
+          expect(gitlab_shell)
+            .to receive(:gitlab_shell_fast_execute)
+            .with([
+              :gitlab_shell_keys_path,
+              'add-key',
+              'key-123',
+              'ssh-rsa foobar'
+            ])
+
+          gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+        end
+      end
+
+      context 'authorized_keys_file set' do
+        it 'calls Gitlab::AuthorizedKeys#add_key with id and key' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+
+          expect(gitlab_authorized_keys)
+            .to receive(:add_key)
+            .with('key-123', 'ssh-rsa foobar')
 
-        gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+          gitlab_shell.add_key('key-123', 'ssh-rsa foobar')
+        end
       end
     end
   end
 
   describe '#batch_add_keys' do
+    let(:keys) { [double(shell_id: 'key-123', key: 'ssh-rsa foobar')] }
+
     context 'when authorized_keys_enabled is true' do
-      it 'instantiates KeyAdder' do
-        expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar')
+      context 'authorized_keys_file not set' do
+        let(:io) { double }
+
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+        end
+
+        context 'valid keys' do
+          before do
+            allow(gitlab_shell)
+              .to receive(:gitlab_shell_keys_path)
+              .and_return(:gitlab_shell_keys_path)
+          end
+
+          it 'calls gitlab-keys with batch-add-keys command' do
+            expect(IO)
+              .to receive(:popen)
+              .with("gitlab_shell_keys_path batch-add-keys", 'w')
+              .and_yield(io)
+
+            expect(io).to receive(:puts).with("key-123\tssh-rsa foobar")
+            expect(gitlab_shell.batch_add_keys(keys)).to be_truthy
+          end
+        end
+
+        context 'invalid keys' do
+          let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] }
+
+          it 'catches failure and returns false' do
+            expect(gitlab_shell.batch_add_keys(keys)).to be_falsey
+          end
+        end
+      end
 
-        gitlab_shell.batch_add_keys do |adder|
-          adder.add_key('key-123', 'ssh-rsa foobar')
+      context 'authorized_keys_file set' do
+        it 'calls Gitlab::AuthorizedKeys#batch_add_keys with keys to be added' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+
+          expect(gitlab_authorized_keys)
+            .to receive(:batch_add_keys)
+            .with(keys)
+
+          gitlab_shell.batch_add_keys(keys)
         end
       end
     end
@@ -103,11 +208,23 @@ describe Gitlab::Shell do
         stub_application_setting(authorized_keys_enabled: false)
       end
 
-      it 'does nothing' do
-        expect_any_instance_of(Gitlab::Shell::KeyAdder).not_to receive(:add_key)
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+        end
+
+        it 'does nothing' do
+          expect(IO).not_to receive(:popen)
+
+          gitlab_shell.batch_add_keys(keys)
+        end
+      end
+
+      context 'authorized_keys_file set' do
+        it 'does nothing' do
+          expect(Gitlab::AuthorizedKeys).not_to receive(:new)
 
-        gitlab_shell.batch_add_keys do |adder|
-          adder.add_key('key-123', 'ssh-rsa foobar')
+          gitlab_shell.batch_add_keys(keys)
         end
       end
     end
@@ -117,11 +234,37 @@ describe Gitlab::Shell do
         stub_application_setting(authorized_keys_enabled: nil)
       end
 
-      it 'instantiates KeyAdder' do
-        expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar')
+      context 'authorized_keys_file not set' do
+        let(:io) { double }
 
-        gitlab_shell.batch_add_keys do |adder|
-          adder.add_key('key-123', 'ssh-rsa foobar')
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
+
+        it 'calls gitlab-keys with batch-add-keys command' do
+          expect(IO)
+            .to receive(:popen)
+            .with("gitlab_shell_keys_path batch-add-keys", 'w')
+            .and_yield(io)
+
+          expect(io).to receive(:puts).with("key-123\tssh-rsa foobar")
+
+          gitlab_shell.batch_add_keys(keys)
+        end
+      end
+
+      context 'authorized_keys_file set' do
+        it 'calls Gitlab::AuthorizedKeys#batch_add_keys with keys to be added' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+
+          expect(gitlab_authorized_keys)
+            .to receive(:batch_add_keys)
+            .with(keys)
+
+          gitlab_shell.batch_add_keys(keys)
         end
       end
     end
@@ -129,13 +272,34 @@ describe Gitlab::Shell do
 
   describe '#remove_key' do
     context 'when authorized_keys_enabled is true' do
-      it 'removes trailing garbage' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
-          [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar']
-        )
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
 
-        gitlab_shell.remove_key('key-123', 'ssh-rsa foobar')
+        it 'calls #gitlab_shell_fast_execute with rm-key command' do
+          expect(gitlab_shell)
+            .to receive(:gitlab_shell_fast_execute)
+            .with([
+              :gitlab_shell_keys_path,
+              'rm-key',
+              'key-123'
+            ])
+
+          gitlab_shell.remove_key('key-123')
+        end
+      end
+
+      context 'authorized_keys_file not set' do
+        it 'calls Gitlab::AuthorizedKeys#rm_key with the key to be removed' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+          expect(gitlab_authorized_keys).to receive(:rm_key).with('key-123')
+
+          gitlab_shell.remove_key('key-123')
+        end
       end
     end
 
@@ -144,10 +308,24 @@ describe Gitlab::Shell do
         stub_application_setting(authorized_keys_enabled: false)
       end
 
-      it 'does nothing' do
-        expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute)
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+        end
+
+        it 'does nothing' do
+          expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute)
 
-        gitlab_shell.remove_key('key-123', 'ssh-rsa foobar')
+          gitlab_shell.remove_key('key-123')
+        end
+      end
+
+      context 'authorized_keys_file set' do
+        it 'does nothing' do
+          expect(Gitlab::AuthorizedKeys).not_to receive(:new)
+
+          gitlab_shell.remove_key('key-123')
+        end
       end
     end
 
@@ -156,232 +334,256 @@ describe Gitlab::Shell do
         stub_application_setting(authorized_keys_enabled: nil)
       end
 
-      it 'removes trailing garbage' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
-          [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar']
-        )
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
+
+        it 'calls #gitlab_shell_fast_execute with rm-key command' do
+          expect(gitlab_shell)
+            .to receive(:gitlab_shell_fast_execute)
+            .with([
+              :gitlab_shell_keys_path,
+              'rm-key',
+              'key-123'
+            ])
 
-        gitlab_shell.remove_key('key-123', 'ssh-rsa foobar')
+          gitlab_shell.remove_key('key-123')
+        end
       end
-    end
 
-    context 'when key content is not given' do
-      it 'calls rm-key with only one argument' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
-          [:gitlab_shell_keys_path, 'rm-key', 'key-123']
-        )
+      context 'authorized_keys_file not set' do
+        it 'calls Gitlab::AuthorizedKeys#rm_key with the key to be removed' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+          expect(gitlab_authorized_keys).to receive(:rm_key).with('key-123')
 
-        gitlab_shell.remove_key('key-123')
+          gitlab_shell.remove_key('key-123')
+        end
       end
     end
   end
 
   describe '#remove_all_keys' do
     context 'when authorized_keys_enabled is true' do
-      it 'removes trailing garbage' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with([:gitlab_shell_keys_path, 'clear'])
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
 
-        gitlab_shell.remove_all_keys
-      end
-    end
+        it 'calls #gitlab_shell_fast_execute with clear command' do
+          expect(gitlab_shell)
+            .to receive(:gitlab_shell_fast_execute)
+            .with([:gitlab_shell_keys_path, 'clear'])
 
-    context 'when authorized_keys_enabled is false' do
-      before do
-        stub_application_setting(authorized_keys_enabled: false)
+          gitlab_shell.remove_all_keys
+        end
       end
 
-      it 'does nothing' do
-        expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute)
+      context 'authorized_keys_file set' do
+        it 'calls Gitlab::AuthorizedKeys#clear' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+          expect(gitlab_authorized_keys).to receive(:clear)
 
-        gitlab_shell.remove_all_keys
+          gitlab_shell.remove_all_keys
+        end
       end
     end
 
-    context 'when authorized_keys_enabled is nil' do
+    context 'when authorized_keys_enabled is false' do
       before do
-        stub_application_setting(authorized_keys_enabled: nil)
+        stub_application_setting(authorized_keys_enabled: false)
       end
 
-      it 'removes trailing garbage' do
-        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-        expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
-          [:gitlab_shell_keys_path, 'clear']
-        )
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+        end
 
-        gitlab_shell.remove_all_keys
-      end
-    end
-  end
+        it 'does nothing' do
+          expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute)
 
-  describe '#remove_keys_not_found_in_db' do
-    context 'when keys are in the file that are not in the DB' do
-      before do
-        gitlab_shell.remove_all_keys
-        gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
-        gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF')
-        @another_key = create(:key) # this one IS in the DB
+          gitlab_shell.remove_all_keys
+        end
       end
 
-      it 'removes the keys' do
-        expect(find_in_authorized_keys_file(1234)).to be_truthy
-        expect(find_in_authorized_keys_file(9876)).to be_truthy
-        expect(find_in_authorized_keys_file(@another_key.id)).to be_truthy
-        gitlab_shell.remove_keys_not_found_in_db
-        expect(find_in_authorized_keys_file(1234)).to be_falsey
-        expect(find_in_authorized_keys_file(9876)).to be_falsey
-        expect(find_in_authorized_keys_file(@another_key.id)).to be_truthy
+      context 'authorized_keys_file set' do
+        it 'does nothing' do
+          expect(Gitlab::AuthorizedKeys).not_to receive(:new)
+
+          gitlab_shell.remove_all_keys
+        end
       end
     end
 
-    context 'when keys there are duplicate keys in the file that are not in the DB' do
+    context 'when authorized_keys_enabled is nil' do
       before do
-        gitlab_shell.remove_all_keys
-        gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
-        gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+        stub_application_setting(authorized_keys_enabled: nil)
       end
 
-      it 'removes the keys' do
-        expect(find_in_authorized_keys_file(1234)).to be_truthy
-        gitlab_shell.remove_keys_not_found_in_db
-        expect(find_in_authorized_keys_file(1234)).to be_falsey
-      end
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          allow(gitlab_shell)
+            .to receive(:gitlab_shell_keys_path)
+            .and_return(:gitlab_shell_keys_path)
+        end
 
-      it 'does not run remove more than once per key (in a batch)' do
-        expect(gitlab_shell).to receive(:remove_key).with('key-1234').once
-        gitlab_shell.remove_keys_not_found_in_db
-      end
-    end
+        it 'calls #gitlab_shell_fast_execute with clear command' do
+          expect(gitlab_shell)
+            .to receive(:gitlab_shell_fast_execute)
+            .with([:gitlab_shell_keys_path, 'clear'])
 
-    context 'when keys there are duplicate keys in the file that ARE in the DB' do
-      before do
-        gitlab_shell.remove_all_keys
-        @key = create(:key)
-        gitlab_shell.add_key(@key.shell_id, @key.key)
+          gitlab_shell.remove_all_keys
+        end
       end
 
-      it 'does not remove the key' do
-        gitlab_shell.remove_keys_not_found_in_db
-        expect(find_in_authorized_keys_file(@key.id)).to be_truthy
-      end
+      context 'authorized_keys_file set' do
+        it 'calls Gitlab::AuthorizedKeys#clear' do
+          expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys)
+          expect(gitlab_authorized_keys).to receive(:clear)
 
-      it 'does not need to run a SELECT query for that batch, on account of that key' do
-        expect_any_instance_of(ActiveRecord::Relation).not_to receive(:pluck)
-        gitlab_shell.remove_keys_not_found_in_db
+          gitlab_shell.remove_all_keys
+        end
       end
     end
+  end
 
-    unless ENV['CI'] # Skip in CI, it takes 1 minute
-      context 'when the first batch can be skipped, but the next batch has keys that are not in the DB' do
+  describe '#remove_keys_not_found_in_db' do
+    context 'when keys are in the file that are not in the DB' do
+      context 'authorized_keys_file not set' do
         before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
           gitlab_shell.remove_all_keys
-          100.times { |i| create(:key) } # first batch is all in the DB
           gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+          gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF')
+          @another_key = create(:key) # this one IS in the DB
         end
 
-        it 'removes the keys not in the DB' do
-          expect(find_in_authorized_keys_file(1234)).to be_truthy
+        it 'removes the keys' do
+          expect(gitlab_shell).to receive(:remove_key).with('key-1234')
+          expect(gitlab_shell).to receive(:remove_key).with('key-9876')
+          expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@another_key.id}")
+
           gitlab_shell.remove_keys_not_found_in_db
-          expect(find_in_authorized_keys_file(1234)).to be_falsey
         end
       end
-    end
-  end
 
-  describe '#batch_read_key_ids' do
-    context 'when there are keys in the authorized_keys file' do
-      before do
-        gitlab_shell.remove_all_keys
-        (1..4).each do |i|
-          gitlab_shell.add_key("key-#{i}", "ssh-rsa ASDFASDF#{i}")
+      context 'authorized_keys_file set' do
+        before do
+          gitlab_shell.remove_all_keys
+          gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+          gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF')
+          @another_key = create(:key) # this one IS in the DB
         end
-      end
 
-      it 'iterates over the key IDs in the file, in batches' do
-        loop_count = 0
-        first_batch = [1, 2]
-        second_batch = [3, 4]
+        it 'removes the keys' do
+          expect(gitlab_shell).to receive(:remove_key).with('key-1234')
+          expect(gitlab_shell).to receive(:remove_key).with('key-9876')
+          expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@another_key.id}")
 
-        gitlab_shell.batch_read_key_ids(batch_size: 2) do |batch|
-          expected = (loop_count == 0 ? first_batch : second_batch)
-          expect(batch).to eq(expected)
-          loop_count += 1
+          gitlab_shell.remove_keys_not_found_in_db
         end
       end
     end
-  end
 
-  describe '#list_key_ids' do
-    context 'when there are keys in the authorized_keys file' do
-      before do
-        gitlab_shell.remove_all_keys
-        (1..4).each do |i|
-          gitlab_shell.add_key("key-#{i}", "ssh-rsa ASDFASDF#{i}")
+    context 'when keys there are duplicate keys in the file that are not in the DB' do
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          gitlab_shell.remove_all_keys
+          gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+          gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+        end
+
+        it 'removes the keys' do
+          expect(gitlab_shell).to receive(:remove_key).with('key-1234')
+
+          gitlab_shell.remove_keys_not_found_in_db
         end
       end
 
-      it 'outputs the key IDs in the file, separated by newlines' do
-        ids = []
-        gitlab_shell.list_key_ids do |io|
-          io.each do |line|
-            ids << line
-          end
+      context 'authorized_keys_file set' do
+        before do
+          gitlab_shell.remove_all_keys
+          gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+          gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
         end
 
-        expect(ids).to eq(%W{1\n 2\n 3\n 4\n})
-      end
-    end
+        it 'removes the keys' do
+          expect(gitlab_shell).to receive(:remove_key).with('key-1234')
 
-    context 'when there are no keys in the authorized_keys file' do
-      before do
-        gitlab_shell.remove_all_keys
+          gitlab_shell.remove_keys_not_found_in_db
+        end
       end
+    end
 
-      it 'outputs nothing, not even an empty string' do
-        ids = []
-        gitlab_shell.list_key_ids do |io|
-          io.each do |line|
-            ids << line
-          end
+    context 'when keys there are duplicate keys in the file that ARE in the DB' do
+      context 'authorized_keys_file not set' do
+        before do
+          stub_gitlab_shell_setting(authorized_keys_file: nil)
+          gitlab_shell.remove_all_keys
+          @key = create(:key)
+          gitlab_shell.add_key(@key.shell_id, @key.key)
         end
 
-        expect(ids).to eq([])
+        it 'does not remove the key' do
+          expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@key.id}")
+
+          gitlab_shell.remove_keys_not_found_in_db
+        end
       end
-    end
-  end
 
-  describe Gitlab::Shell::KeyAdder do
-    describe '#add_key' do
-      it 'removes trailing garbage' do
-        io = spy(:io)
-        adder = described_class.new(io)
+      context 'authorized_keys_file set' do
+        before do
+          gitlab_shell.remove_all_keys
+          @key = create(:key)
+          gitlab_shell.add_key(@key.shell_id, @key.key)
+        end
 
-        adder.add_key('key-42', "ssh-rsa foo bar\tbaz")
+        it 'does not remove the key' do
+          expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@key.id}")
 
-        expect(io).to have_received(:puts).with("key-42\tssh-rsa foo")
+          gitlab_shell.remove_keys_not_found_in_db
+        end
       end
+    end
 
-      it 'handles multiple spaces in the key' do
-        io = spy(:io)
-        adder = described_class.new(io)
+    unless ENV['CI'] # Skip in CI, it takes 1 minute
+      context 'when the first batch can be skipped, but the next batch has keys that are not in the DB' do
+        context 'authorized_keys_file not set' do
+          before do
+            stub_gitlab_shell_setting(authorized_keys_file: nil)
+            gitlab_shell.remove_all_keys
+            100.times { |i| create(:key) } # first batch is all in the DB
+            gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+          end
 
-        adder.add_key('key-42', "ssh-rsa  foo")
+          it 'removes the keys not in the DB' do
+            expect(gitlab_shell).to receive(:remove_key).with('key-1234')
 
-        expect(io).to have_received(:puts).with("key-42\tssh-rsa foo")
-      end
+            gitlab_shell.remove_keys_not_found_in_db
+          end
+        end
 
-      it 'raises an exception if the key contains a tab' do
-        expect do
-          described_class.new(StringIO.new).add_key('key-42', "ssh-rsa\tfoobar")
-        end.to raise_error(Gitlab::Shell::Error)
-      end
+        context 'authorized_keys_file set' do
+          before do
+            gitlab_shell.remove_all_keys
+            100.times { |i| create(:key) } # first batch is all in the DB
+            gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF')
+          end
+
+          it 'removes the keys not in the DB' do
+            expect(gitlab_shell).to receive(:remove_key).with('key-1234')
 
-      it 'raises an exception if the key contains a newline' do
-        expect do
-          described_class.new(StringIO.new).add_key('key-42', "ssh-rsa foobar\nssh-rsa pawned")
-        end.to raise_error(Gitlab::Shell::Error)
+            gitlab_shell.remove_keys_not_found_in_db
+          end
+        end
       end
     end
   end
@@ -566,12 +768,4 @@ describe Gitlab::Shell do
       end
     end
   end
-
-  def find_in_authorized_keys_file(key_id)
-    gitlab_shell.batch_read_key_ids do |ids|
-      return true if ids.include?(key_id) # rubocop:disable Cop/AvoidReturnFromBlocks
-    end
-
-    false
-  end
 end