diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-08-03 19:22:46 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-08-03 19:22:46 +0000 |
commit | 4a9d52652a0081bb572075e8cc7c587db956099e (patch) | |
tree | 5926eb9bff5426afcdeb350d500bdf3895cc145f | |
parent | a7d2fed0a64ec6271cced4dffe24021907e8ccd7 (diff) | |
parent | 784221bdb261c5edf9449f10e69ed8ebb5c98c03 (diff) | |
download | gitlab-shell-4a9d52652a0081bb572075e8cc7c587db956099e.tar.gz |
Merge branch 'authorized-keys-permission-check' into 'master'
Improve authorized_keys check
The old check only looked if authorized_keys exists. With this change, we look
whether we can actually open the file for reading and writing. When this fails
we try to print useful diagnostic information.
See merge request !79
-rwxr-xr-x | bin/check | 8 | ||||
-rw-r--r-- | lib/gitlab_keys.rb | 13 | ||||
-rw-r--r-- | spec/gitlab_keys_spec.rb | 21 |
3 files changed, 36 insertions, 6 deletions
@@ -19,14 +19,12 @@ rescue GitlabNet::ApiUnreachableError abort "FAILED: Failed to connect to internal API" end - -puts "\nCheck directories and files: " - config = GitlabConfig.new abort("ERROR: missing option in config.yml") unless config.auth_file -print "\t#{config.auth_file}: " -if File.exists?(config.auth_file) + +print "\nAccess to #{config.auth_file}: " +if system(File.dirname(__FILE__) + '/gitlab-keys', 'check-permissions') print 'OK' else abort "FAILED" diff --git a/lib/gitlab_keys.rb b/lib/gitlab_keys.rb index e1b62ad..c719e8d 100644 --- a/lib/gitlab_keys.rb +++ b/lib/gitlab_keys.rb @@ -21,6 +21,7 @@ class GitlabKeys when 'rm-key'; rm_key when 'list-keys'; puts list_keys when 'clear'; clear + when 'check-permissions'; check_permissions else $logger.warn "Attempt to execute invalid gitlab-keys command #{@command.inspect}." puts 'not allowed' @@ -92,6 +93,18 @@ class GitlabKeys true end + def check_permissions + open_auth_file('r+') { 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| diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb index 5afa467..d761cc4 100644 --- a/spec/gitlab_keys_spec.rb +++ b/spec/gitlab_keys_spec.rb @@ -15,7 +15,6 @@ describe GitlabKeys do it { gitlab_keys.instance_variable_get(:@key_id).should == 'key-741' } end - describe :add_key do let(:gitlab_keys) { build_gitlab_keys('add-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') } @@ -145,6 +144,20 @@ describe GitlabKeys do 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 + gitlab_keys.should_receive(:open_auth_file).and_raise("imaginary error") + expect(gitlab_keys.exec).to eq(false) + end + end + describe :exec do it 'add-key arg should execute add_key method' do gitlab_keys = build_gitlab_keys('add-key') @@ -170,6 +183,12 @@ describe GitlabKeys do gitlab_keys.exec end + it 'check-permissions arg should execute check_permissions method' do + gitlab_keys = build_gitlab_keys('check-permissions') + gitlab_keys.should_receive(:check_permissions) + gitlab_keys.exec + end + it 'should puts message if unknown command arg' do gitlab_keys = build_gitlab_keys('change-key') gitlab_keys.should_receive(:puts).with('not allowed') |