Commit 983dcba3 authored by Patrick Bajao's avatar Patrick Bajao

Handle cases when authorized_keys doesn't exist

Modify Gitlab::AuthorizedKeys#rm_key and #list_key_ids to
handle cases when an `Errno::ENOENT` is raised.

Also added test cases in spec to ensure each method will
still work when authorized_keys doesn't exist.
parent dd43abec
...@@ -68,6 +68,8 @@ module Gitlab ...@@ -68,6 +68,8 @@ module Gitlab
end end
true true
rescue Errno::ENOENT
false
end end
# Clear the authorized_keys file # Clear the authorized_keys file
...@@ -96,6 +98,8 @@ module Gitlab ...@@ -96,6 +98,8 @@ module Gitlab
end end
end end
end end
rescue Errno::ENOENT
[]
end end
private private
......
...@@ -7,15 +7,31 @@ describe Gitlab::AuthorizedKeys do ...@@ -7,15 +7,31 @@ describe Gitlab::AuthorizedKeys do
subject { described_class.new(logger) } subject { described_class.new(logger) }
after do
delete_authorized_keys_file
end
describe '#add_key' do 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
create_authorized_keys_fixture before 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" create_authorized_keys_fixture
end
expect(logger).to receive(:info).with('Adding key (key-741): ssh-rsa AAAAB3NzaDAxx2E')
expect(subject.add_key('key-741', 'ssh-rsa AAAAB3NzaDAxx2E trailing garbage')) it "adds a line at the end of the file and strips trailing garbage" do
.to be_truthy 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(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line}\n")
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
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
end end
...@@ -27,63 +43,103 @@ describe Gitlab::AuthorizedKeys do ...@@ -27,63 +43,103 @@ describe Gitlab::AuthorizedKeys do
] ]
end end
before do context 'authorized_keys file exists' do
create_authorized_keys_fixture before do
end create_authorized_keys_fixture
end
it "adds lines at the end of the file" do 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_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" 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-12): ssh-dsa ASDFASGADG')
expect(logger).to receive(:info).with('Adding key (key-123): ssh-rsa GFDGDFSGSDFG') expect(logger).to receive(:info).with('Adding key (key-123): ssh-rsa GFDGDFSGSDFG')
expect(subject.batch_add_keys(keys)).to be_truthy 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") expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line1}\n#{auth_line2}\n")
end end
context "invalid key" do context "invalid key" do
let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] } let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] }
it "doesn't add keys" do it "doesn't add keys" do
expect(subject.batch_add_keys(keys)).to be_falsey expect(subject.batch_add_keys(keys)).to be_falsey
expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n") expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n")
end
end
end
context 'authorized_keys file does not exist' do
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 end
end end
describe '#rm_key' do describe '#rm_key' do
it "removes the right line" do context 'authorized_keys file exists' do
create_authorized_keys_fixture before do
other_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E" create_authorized_keys_fixture
delete_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E" end
erased_line = delete_line.gsub(/./, '#')
File.open(tmp_authorized_keys_path, 'a') do |auth_file| it "removes the right line" do
auth_file.puts delete_line other_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E"
auth_file.puts other_line delete_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E"
end erased_line = delete_line.gsub(/./, '#')
File.open(tmp_authorized_keys_path, 'a') do |auth_file|
expect(logger).to receive(:info).with('Removing key (key-741)') auth_file.puts delete_line
expect(subject.rm_key('key-741')).to be_truthy auth_file.puts other_line
expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{erased_line}\n#{other_line}\n") 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
it 'returns false' do
expect(subject.rm_key('key-741')).to be_falsey
end
end end
end end
describe '#clear' do describe '#clear' do
it "should return true" do context 'authorized_keys file exists' do
expect(subject.clear).to be_truthy before do
create_authorized_keys_fixture
end
it "returns true" do
expect(subject.clear).to be_truthy
end
end
context 'authorized_keys file does not exist' do
it "still returns true" do
expect(subject.clear).to be_truthy
end
end end
end end
describe '#list_key_ids' do describe '#list_key_ids' do
before do context 'authorized_keys file exists' do
create_authorized_keys_fixture( before do
existing_content: create_authorized_keys_fixture(
"key-1\tssh-dsa AAA\nkey-2\tssh-rsa BBB\nkey-3\tssh-rsa CCC\nkey-9000\tssh-rsa DDD\n" 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 end
it 'returns array of key IDs' do context 'authorized_keys file does not exist' do
expect(subject.list_key_ids).to eq([1, 2, 3, 9000]) it 'returns an empty array' do
expect(subject.list_key_ids).to be_empty
end
end end
end end
...@@ -92,6 +148,10 @@ describe Gitlab::AuthorizedKeys do ...@@ -92,6 +148,10 @@ describe Gitlab::AuthorizedKeys do
File.open(tmp_authorized_keys_path, 'w') { |file| file.puts(existing_content) } File.open(tmp_authorized_keys_path, 'w') { |file| file.puts(existing_content) }
end 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 def tmp_authorized_keys_path
Gitlab.config.gitlab_shell.authorized_keys_file Gitlab.config.gitlab_shell.authorized_keys_file
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment