From cb9b5447eb8cb8ae6535ebf90e5a5e9e724721bd Mon Sep 17 00:00:00 2001 From: Patrick Bajao Date: Fri, 23 Aug 2019 14:15:33 +0800 Subject: Remove gitlab-keys script --- README.md | 18 --- bin/check | 10 -- bin/gitlab-keys | 27 ---- lib/gitlab_keys.rb | 147 +--------------------- spec/gitlab_keys_spec.rb | 312 ----------------------------------------------- 5 files changed, 1 insertion(+), 513 deletions(-) delete mode 100755 bin/gitlab-keys diff --git a/README.md b/README.md index 806de08..915df1a 100644 --- a/README.md +++ b/README.md @@ -76,24 +76,6 @@ Checks if GitLab API access and redis via internal API can be reached: make check -## Keys - -Add key: - - ./bin/gitlab-keys add-key key-782 "ssh-rsa AAAAx321..." - -Remove key: - - ./bin/gitlab-keys rm-key key-23 "ssh-rsa AAAAx321..." - -List all keys: - - ./bin/gitlab-keys list-keys - -Remove all keys from authorized_keys file: - - ./bin/gitlab-keys clear - ## Testing Run Ruby and Golang tests: diff --git a/bin/check b/bin/check index 2acee63..6d42f9b 100755 --- a/bin/check +++ b/bin/check @@ -29,14 +29,4 @@ rescue GitlabNet::ApiUnreachableError abort "FAILED: Failed to connect to internal API" end -config = GitlabConfig.new - -abort("ERROR: missing option in config.yml") unless config.auth_file - -print "Access to #{config.auth_file}: " -if system(File.dirname(__FILE__) + '/gitlab-keys', 'check-permissions') - print 'OK' -else - abort "FAILED" -end puts "\n" diff --git a/bin/gitlab-keys b/bin/gitlab-keys deleted file mode 100755 index 9eb1950..0000000 --- a/bin/gitlab-keys +++ /dev/null @@ -1,27 +0,0 @@ -#!/usr/bin/env ruby - -require_relative '../lib/gitlab_init' - -# -# GitLab Keys shell. Add/remove keys from ~/.ssh/authorized_keys -# -# Ex. -# /bin/gitlab-keys add-key key-782 "ssh-rsa AAAAx321..." -# -# printf "key-782\tssh-rsa AAAAx321...\n" | /bin/gitlab-keys batch-add-keys -# -# /bin/gitlab-keys rm-key key-23 "ssh-rsa AAAAx321..." -# -# /bin/gitlab-keys list-keys -# -# /bin/gitlab-keys clear -# - -require File.join(ROOT_PATH, 'lib', 'gitlab_keys') - -# Return non-zero if command execution was not successful -if GitlabKeys.new.exec - exit 0 -else - exit 1 -end diff --git a/lib/gitlab_keys.rb b/lib/gitlab_keys.rb index 85ca797..f583ede 100644 --- a/lib/gitlab_keys.rb +++ b/lib/gitlab_keys.rb @@ -4,11 +4,9 @@ require_relative 'gitlab_config' require_relative 'gitlab_logger' require_relative 'gitlab_metrics' -class GitlabKeys # rubocop:disable Metrics/ClassLength +module GitlabKeys class KeyError < StandardError; end - attr_accessor :auth_file, :key - def self.command(whatever) "#{ROOT_PATH}/bin/gitlab-shell #{whatever}" end @@ -44,147 +42,4 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength whatever_line(command_key(username_key_id), principal) end - - def initialize - @command = ARGV.shift - @key_id = ARGV.shift - key = ARGV.shift - @key = key.dup if key - @auth_file = GitlabConfig.new.auth_file - end - - def exec - GitlabMetrics.measure("command-#{@command}") do - case @command - when 'add-key' - add_key - when 'batch-add-keys' - batch_add_keys - when 'rm-key' - rm_key - when 'list-keys' - list_keys - when 'list-key-ids' - list_key_ids - when 'clear' - clear - when 'check-permissions' - check_permissions - else - $logger.warn('Attempt to execute invalid gitlab-keys command', command: @command.inspect) - puts 'not allowed' - false - end - end - end - - protected - - def add_key - lock do - $logger.info('Adding key', key_id: @key_id, public_key: @key) - auth_line = self.class.key_line(@key_id, @key) - open_auth_file('a') { |file| file.puts(auth_line) } - end - true - end - - def list_keys - $logger.info 'Listing all keys' - keys = '' - File.readlines(auth_file).each do |line| - # key_id & public_key - # command=".../bin/gitlab-shell key-741" ... ssh-rsa AAAAB3NzaDAxx2E\n - # ^^^^^^^ ^^^^^^^^^^^^^^^ - matches = /^command=\".+?\s+(.+?)\".+?(?:ssh|ecdsa)-.*?\s(.+)\s*.*\n*$/.match(line) - keys << "#{matches[1]} #{matches[2]}\n" unless matches.nil? - end - puts keys - end - - def list_key_ids - $logger.info 'Listing all key IDs' - open_auth_file('r') do |f| - f.each_line do |line| - matchd = line.match(/key-(\d+)/) - next unless matchd - puts matchd[1] - end - end - end - - def batch_add_keys - lock(300) do # Allow 300 seconds (5 minutes) for batch_add_keys - open_auth_file('a') do |file| - stdin.each_line do |input| - tokens = input.strip.split("\t") - abort("#{$0}: invalid input #{input.inspect}") unless tokens.count == 2 - key_id, public_key = tokens - $logger.info('Adding key', key_id: key_id, public_key: public_key) - file.puts(self.class.key_line(key_id, public_key)) - end - end - end - true - end - - def stdin - $stdin - end - - def rm_key - lock do - $logger.info('Removing key', key_id: @key_id) - open_auth_file('r+') do |f| - while line = f.gets # rubocop:disable Lint/AssignmentInCondition - next unless line.start_with?("command=\"#{self.class.command_key(@key_id)}\"") - f.seek(-line.length, IO::SEEK_CUR) - # Overwrite the line with #'s. Because the 'line' variable contains - # a terminating '\n', we write line.length - 1 '#' characters. - f.write('#' * (line.length - 1)) - end - end - end - true - end - - def clear - open_auth_file('w') { |file| file.puts '# Managed by gitlab-shell' } - true - end - - def check_permissions - open_auth_file(File::RDWR | File::CREAT) { true } - rescue => ex - puts "error: could not open #{auth_file}: #{ex}" - if File.exist?(auth_file) - system('ls', '-l', auth_file) - else - # Maybe the parent directory is not writable? - system('ls', '-ld', File.dirname(auth_file)) - end - false - end - - def lock(timeout = 10) - File.open(lock_file, "w+") do |f| - begin - f.flock File::LOCK_EX - Timeout.timeout(timeout) { yield } - ensure - f.flock File::LOCK_UN - end - end - end - - def lock_file - @lock_file ||= auth_file + '.lock' - end - - def open_auth_file(mode) - open(auth_file, mode, 0o600) do |file| - file.chmod(0o600) - yield file - end - end end diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb index 86128f6..1853918 100644 --- a/spec/gitlab_keys_spec.rb +++ b/spec/gitlab_keys_spec.rb @@ -61,316 +61,4 @@ describe GitlabKeys do expect { described_class.principal_line('username-someuser', "sshUsers\nloginUsers") }.to raise_error(described_class::KeyError) end end - - describe :initialize do - let(:gitlab_keys) { build_gitlab_keys('add-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') } - - it { expect(gitlab_keys.key).to eq('ssh-rsa AAAAB3NzaDAxx2E') } - it { expect(gitlab_keys.instance_variable_get(:@command)).to eq('add-key') } - it { expect(gitlab_keys.instance_variable_get(:@key_id)).to eq('key-741') } - end - - describe :add_key do - let(:gitlab_keys) { build_gitlab_keys('add-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') } - - it "adds a line at the end of the file" do - create_authorized_keys_fixture - gitlab_keys.send :add_key - auth_line = "command=\"#{ROOT_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") - end - - context "without file writing" do - before { allow(gitlab_keys).to receive(:open) } - before { create_authorized_keys_fixture } - - it "should log an add-key event" do - expect($logger).to receive(:info).with("Adding key", {:key_id=>"key-741", :public_key=>"ssh-rsa AAAAB3NzaDAxx2E"}) - gitlab_keys.send :add_key - end - - it "should return true" do - expect(gitlab_keys.send(:add_key)).to be_truthy - end - end - end - - describe ':list_keys' do - let(:gitlab_keys) { build_gitlab_keys('list_keys') } - let(:key_data) { "%s\n%s\n" % [described_class.key_line('key-741', 'ssh-rsa AAAAB3NzaDAxx2E'), described_class.key_line('key-742', 'ssh-rsa AAAAB3NzaDAxx2E')] } - let(:key_output) { "key-741 AAAAB3NzaDAxx2E\nkey-742 AAAAB3NzaDAxx2E\n" } - - before do - create_authorized_keys_fixture( - existing_content: - key_data - ) - end - - it 'outputs the keys with IDs, separated by newlines' do - expect { gitlab_keys.send(:list_keys) }.to output(key_output).to_stdout - end - end - - describe :list_key_ids do - let(:gitlab_keys) { build_gitlab_keys('list-key-ids') } - 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 'outputs the key IDs, separated by newlines' do - expect { gitlab_keys.send(:list_key_ids) }.to output("1\n2\n3\n9000\n").to_stdout - end - end - - describe :batch_add_keys do - let(:gitlab_keys) { build_gitlab_keys('batch-add-keys') } - let(:fake_stdin) { StringIO.new("key-12\tssh-dsa ASDFASGADG\nkey-123\tssh-rsa GFDGDFSGSDFG\n", 'r') } - before do - create_authorized_keys_fixture - allow(gitlab_keys).to receive(:stdin).and_return(fake_stdin) - end - - it "adds lines at the end of the file" do - gitlab_keys.send :batch_add_keys - auth_line1 = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-12\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-dsa ASDFASGADG" - auth_line2 = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-123\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa GFDGDFSGSDFG" - expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line1}\n#{auth_line2}\n") - end - - context "with invalid input" do - let(:fake_stdin) { StringIO.new("key-12\tssh-dsa ASDFASGADG\nkey-123\tssh-rsa GFDGDFSGSDFG\nfoo\tbar\tbaz\n", 'r') } - - it "aborts" do - expect(gitlab_keys).to receive(:abort) - gitlab_keys.send :batch_add_keys - end - end - - context "without file writing" do - before do - expect(gitlab_keys).to receive(:open).and_yield(double(:file, puts: nil, chmod: nil)) - end - - it "should log an add-key event" do - expect($logger).to receive(:info).with("Adding key", key_id: 'key-12', public_key: "ssh-dsa ASDFASGADG") - expect($logger).to receive(:info).with("Adding key", key_id: 'key-123', public_key: "ssh-rsa GFDGDFSGSDFG") - gitlab_keys.send :batch_add_keys - end - - it "should return true" do - expect(gitlab_keys.send(:batch_add_keys)).to be_truthy - end - end - end - - describe :stdin do - let(:gitlab_keys) { build_gitlab_keys } - subject { gitlab_keys.send :stdin } - before { $stdin = 1 } - - it { is_expected.to equal(1) } - end - - describe :rm_key do - let(:gitlab_keys) { build_gitlab_keys('rm-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') } - - it "removes the right line" do - create_authorized_keys_fixture - other_line = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E" - delete_line = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E" - open(tmp_authorized_keys_path, 'a') do |auth_file| - auth_file.puts delete_line - auth_file.puts other_line - end - gitlab_keys.send :rm_key - erased_line = delete_line.gsub(/./, '#') - expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{erased_line}\n#{other_line}\n") - end - - context "without file writing" do - before do - allow(gitlab_keys).to receive(:open) - allow(gitlab_keys).to receive(:lock).and_yield - end - - it "should log an rm-key event" do - expect($logger).to receive(:info).with("Removing key", key_id: "key-741") - gitlab_keys.send :rm_key - end - - it "should return true" do - expect(gitlab_keys.send(:rm_key)).to be_truthy - end - end - - context 'without key content' do - let(:gitlab_keys) { build_gitlab_keys('rm-key', 'key-741') } - - it "removes the right line by key ID" do - create_authorized_keys_fixture - other_line = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E" - delete_line = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E" - open(tmp_authorized_keys_path, 'a') do |auth_file| - auth_file.puts delete_line - auth_file.puts other_line - end - gitlab_keys.send :rm_key - erased_line = delete_line.gsub(/./, '#') - expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{erased_line}\n#{other_line}\n") - end - end - end - - describe :clear do - let(:gitlab_keys) { build_gitlab_keys('clear') } - - it "should return true" do - allow(gitlab_keys).to receive(:open) - expect(gitlab_keys.send(:clear)).to be_truthy - end - end - - describe :check_permissions do - let(:gitlab_keys) { build_gitlab_keys('check-permissions') } - - it 'returns true when the file can be opened' do - create_authorized_keys_fixture - expect(gitlab_keys.exec).to eq(true) - end - - it 'returns false if opening raises an exception' do - expect(gitlab_keys).to receive(:open_auth_file).and_raise("imaginary error") - expect(gitlab_keys.exec).to eq(false) - end - - it 'creates the keys file if it does not exist' do - create_authorized_keys_fixture - FileUtils.rm(tmp_authorized_keys_path) - expect(gitlab_keys.exec).to eq(true) - expect(File.exist?(tmp_authorized_keys_path)).to eq(true) - end - end - - describe :exec do - it 'add-key arg should execute add_key method' do - gitlab_keys = build_gitlab_keys('add-key') - expect(gitlab_keys).to receive(:add_key) - gitlab_keys.exec - end - - it 'batch-add-keys arg should execute batch_add_keys method' do - gitlab_keys = build_gitlab_keys('batch-add-keys') - expect(gitlab_keys).to receive(:batch_add_keys) - gitlab_keys.exec - end - - it 'rm-key arg should execute rm_key method' do - gitlab_keys = build_gitlab_keys('rm-key') - expect(gitlab_keys).to receive(:rm_key) - gitlab_keys.exec - end - - it 'clear arg should execute clear method' do - gitlab_keys = build_gitlab_keys('clear') - expect(gitlab_keys).to receive(:clear) - gitlab_keys.exec - end - - it 'check-permissions arg should execute check_permissions method' do - gitlab_keys = build_gitlab_keys('check-permissions') - expect(gitlab_keys).to receive(:check_permissions) - gitlab_keys.exec - end - - it 'should puts message if unknown command arg' do - gitlab_keys = build_gitlab_keys('change-key') - expect(gitlab_keys).to receive(:puts).with('not allowed') - gitlab_keys.exec - end - - it 'should log a warning on unknown commands' do - gitlab_keys = build_gitlab_keys('nooope') - allow(gitlab_keys).to receive(:puts).and_return(nil) - expect($logger).to receive(:warn).with("Attempt to execute invalid gitlab-keys command", command: '"nooope"') - gitlab_keys.exec - end - end - - describe :lock do - before do - allow_any_instance_of(GitlabKeys).to receive(:lock_file).and_return(tmp_lock_file_path) - end - - it "should raise exception if operation lasts more then timeout" do - key = GitlabKeys.new - expect do - key.send :lock, 1 do - sleep 2 - end - end.to raise_error(Timeout::Error, 'execution expired') - end - - it "should actually lock file" do - $global = "" - key = GitlabKeys.new - - thr1 = Thread.new do - key.send :lock do - # Put bigger sleep here to test if main thread will - # wait for lock file released before executing code - sleep 1 - $global << "foo" - end - end - - # make sure main thread start lock command after - # thread above - sleep 0.5 - - key.send :lock do - $global << "bar" - end - - thr1.join - expect($global).to eq("foobar") - end - end - - def build_gitlab_keys(*args) - argv(*args) - GitlabKeys.new - end - - def argv(*args) - args.each_with_index do |arg, i| - ARGV[i] = arg.freeze - end - end - - def create_authorized_keys_fixture(existing_content: 'existing content') - FileUtils.mkdir_p(File.dirname(tmp_authorized_keys_path)) - open(tmp_authorized_keys_path, 'w') { |file| file.puts(existing_content) } - allow(gitlab_keys).to receive(:auth_file).and_return(tmp_authorized_keys_path) - end - - def tmp_authorized_keys_path - File.join(ROOT_PATH, 'tmp', 'authorized_keys') - end - - def tmp_lock_file_path - tmp_authorized_keys_path + '.lock' - end - - def capture_stdout(&blk) - old = $stdout - $stdout = fake = StringIO.new - blk.call - fake.string - ensure - $stdout = old - end end -- cgit v1.2.1