diff options
author | Ash McKenzie <amckenzie@gitlab.com> | 2018-07-31 21:06:56 +1000 |
---|---|---|
committer | Ash McKenzie <amckenzie@gitlab.com> | 2018-08-01 10:12:09 +1000 |
commit | 4c4d9f5ef4a2e3ac16d0b02e18b19ba513849f57 (patch) | |
tree | da1206876526db68f4484dd34ea9c00ae08ebb21 | |
parent | 2f733baacdf5d0dca98276cc9b6e895097d5e8d2 (diff) | |
download | gitlab-shell-4c4d9f5ef4a2e3ac16d0b02e18b19ba513849f57.tar.gz |
Use actor when we don't know if it's a Key or User
* Use gl_id when we don't know if it's a key-X or user-X
* Use Actor.new_from(gl_id) which will figure out if it's a Key or User
* Use key_str when we're referring to key-X as key_id is confusing
-rw-r--r-- | lib/action/api_2fa_recovery.rb | 12 | ||||
-rw-r--r-- | lib/action/base.rb | 11 | ||||
-rw-r--r-- | lib/action/git_lfs_authenticate.rb | 10 | ||||
-rw-r--r-- | lib/action/gitaly.rb | 18 | ||||
-rw-r--r-- | lib/gitlab_access.rb | 19 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 29 | ||||
-rw-r--r-- | lib/gitlab_post_receive.rb | 18 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 26 | ||||
-rw-r--r-- | spec/action/api_2fa_recovery.rb_spec.rb | 4 | ||||
-rw-r--r-- | spec/action/git_lfs_authenticate_spec.rb | 5 | ||||
-rw-r--r-- | spec/action/gitaly_spec.rb | 12 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 64 | ||||
-rw-r--r-- | spec/gitlab_post_receive_spec.rb | 4 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 24 |
14 files changed, 137 insertions, 119 deletions
diff --git a/lib/action/api_2fa_recovery.rb b/lib/action/api_2fa_recovery.rb index 827f8aa..5597ff0 100644 --- a/lib/action/api_2fa_recovery.rb +++ b/lib/action/api_2fa_recovery.rb @@ -3,8 +3,8 @@ require_relative '../gitlab_logger' module Action class API2FARecovery < Base - def initialize(key_id) - @key_id = key_id + def initialize(key) + @key = key end def execute(_, _) @@ -13,7 +13,7 @@ module Action private - attr_reader :key_id + attr_reader :key def continue?(question) puts "#{question} (yes/no)" @@ -34,10 +34,10 @@ module Action return end - resp = api.two_factor_recovery_codes(key_id) + resp = api.two_factor_recovery_codes(key.key_id) if resp['success'] codes = resp['recovery_codes'].join("\n") - $logger.info('API 2FA recovery success', user: user.log_username) + $logger.info('API 2FA recovery success', user: key.log_username) puts "Your two-factor authentication recovery codes are:\n\n" \ "#{codes}\n\n" \ "During sign in, use one of the codes above when prompted for\n" \ @@ -45,7 +45,7 @@ module Action "a new device so you do not lose access to your account again." true else - $logger.info('API 2FA recovery error', user: user.log_username) + $logger.info('API 2FA recovery error', user: key.log_username) puts "An error occurred while trying to generate new recovery codes.\n" \ "#{resp['message']}" end diff --git a/lib/action/base.rb b/lib/action/base.rb index 1f24c8c..fe8c836 100644 --- a/lib/action/base.rb +++ b/lib/action/base.rb @@ -3,18 +3,19 @@ require 'json' require_relative '../gitlab_config' require_relative '../gitlab_net' require_relative '../gitlab_metrics' -require_relative '../user' module Action class Base + def initialize + raise NotImplementedError + end + def self.create_from_json(_) raise NotImplementedError end private - attr_reader :key_id - def config @config ||= GitlabConfig.new end @@ -22,9 +23,5 @@ module Action def api @api ||= GitlabNet.new end - - def user - @user ||= User.new(key_id, audit_usernames: config.audit_usernames) - end end end diff --git a/lib/action/git_lfs_authenticate.rb b/lib/action/git_lfs_authenticate.rb index d38d845..218c71e 100644 --- a/lib/action/git_lfs_authenticate.rb +++ b/lib/action/git_lfs_authenticate.rb @@ -3,15 +3,15 @@ require_relative '../gitlab_logger' module Action class GitLFSAuthenticate < Base - def initialize(key_id, repo_name) - @key_id = key_id + def initialize(key, repo_name) + @key = key @repo_name = repo_name end def execute(_, _) GitlabMetrics.measure('lfs-authenticate') do - $logger.info('Processing LFS authentication', user: user.log_username) - lfs_access = api.lfs_authenticate(key_id, repo_name) + $logger.info('Processing LFS authentication', user: key.log_username) + lfs_access = api.lfs_authenticate(key.key_id, repo_name) return unless lfs_access puts lfs_access.authentication_payload @@ -21,6 +21,6 @@ module Action private - attr_reader :key_id, :repo_name + attr_reader :key, :repo_name end end diff --git a/lib/action/gitaly.rb b/lib/action/gitaly.rb index 65397e6..569a1b7 100644 --- a/lib/action/gitaly.rb +++ b/lib/action/gitaly.rb @@ -11,16 +11,16 @@ module Action 'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack') }.freeze - def initialize(key_id, gl_repository, gl_username, repository_path, gitaly) - @key_id = key_id + def initialize(actor, gl_repository, gl_username, repository_path, gitaly) + @actor = actor @gl_repository = gl_repository @gl_username = gl_username @repository_path = repository_path @gitaly = gitaly end - def self.create_from_json(key_id, json) - new(key_id, + def self.create_from_json(actor, json) + new(actor, json['gl_repository'], json['gl_username'], json['repository_path'], @@ -31,13 +31,13 @@ module Action raise ArgumentError, REPOSITORY_PATH_NOT_PROVIDED unless repository_path raise InvalidRepositoryPathError unless valid_repository? - $logger.info('Performing Gitaly command', user: user.log_username) + $logger.info('Performing Gitaly command', user: actor.log_username) process(command, args) end private - attr_reader :gl_repository, :gl_username, :repository_path, :gitaly + attr_reader :actor, :gl_repository, :gl_username, :repository_path, :gitaly def process(command, args) executable = command @@ -50,7 +50,7 @@ module Action end args_string = [File.basename(executable), *args].join(' ') - $logger.info('executing git command', command: args_string, user: user.log_username) + $logger.info('executing git command', command: args_string, user: actor.log_username) exec_cmd(executable, *args) end @@ -77,7 +77,7 @@ module Action 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'LANG' => ENV['LANG'], - 'GL_ID' => key_id, + 'GL_ID' => actor.identifier, 'GL_PROTOCOL' => GitlabNet::GL_PROTOCOL, 'GL_REPOSITORY' => gl_repository, 'GL_USERNAME' => gl_username @@ -90,7 +90,7 @@ module Action { 'repository' => gitaly['repository'], 'gl_repository' => gl_repository, - 'gl_id' => key_id, + 'gl_id' => actor.identifier, 'gl_username' => gl_username } end diff --git a/lib/gitlab_access.rb b/lib/gitlab_access.rb index a1a8b93..6683ee7 100644 --- a/lib/gitlab_access.rb +++ b/lib/gitlab_access.rb @@ -1,6 +1,7 @@ require 'json' require_relative 'errors' +require_relative 'actor' require_relative 'gitlab_init' require_relative 'gitlab_net' require_relative 'names_helper' @@ -10,17 +11,17 @@ require_relative 'object_dirs_helper' class GitlabAccess include NamesHelper - def initialize(gl_repository, repo_path, key_id, changes, protocol) + def initialize(gl_repository, repo_path, gl_id, changes, protocol) @gl_repository = gl_repository @repo_path = repo_path.strip - @key_id = key_id + @gl_id = gl_id @changes = changes.lines @protocol = protocol end def exec GitlabMetrics.measure('check-access:git-receive-pack') do - api.check_access('git-receive-pack', gl_repository, repo_path, key_id, changes, protocol, env: ObjectDirsHelper.all_attributes.to_json) + api.check_access('git-receive-pack', gl_repository, repo_path, actor, changes, protocol, env: ObjectDirsHelper.all_attributes.to_json) end true rescue GitlabNet::ApiUnreachableError @@ -33,9 +34,17 @@ class GitlabAccess private - attr_reader :gl_repository, :repo_path, :key_id, :changes, :protocol + attr_reader :gl_repository, :repo_path, :gl_id, :changes, :protocol def api - GitlabNet.new + @api ||= GitlabNet.new + end + + def config + @config ||= GitlabConfig.new + end + + def actor + @actor ||= Actor.new_from(gl_id, audit_usernames: config.audit_usernames) end end diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 29a91f4..34e2ff6 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -14,34 +14,34 @@ class GitlabNet GL_PROTOCOL = 'ssh'.freeze API_INACCESSIBLE_ERROR = 'API is not accessible'.freeze - def check_access(cmd, gl_repository, repo, key_id, changes, protocol = GL_PROTOCOL, env: {}) + def check_access(cmd, gl_repository, repo, actor, changes, protocol = GL_PROTOCOL, env: {}) changes = changes.join("\n") unless changes.is_a?(String) params = { action: cmd, changes: changes, gl_repository: gl_repository, - key_id: key_id.gsub('key-', ''), project: sanitize_path(repo), protocol: protocol, env: env } + params[actor.class.identifier_key.to_sym] = actor.id + resp = post("#{internal_api_endpoint}/allowed", params) - determine_action(key_id, resp) + determine_action(actor, resp) end - def discover(key) - key_id = key.gsub("key-", "") + def discover(key_id) resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}") JSON.parse(resp.body) rescue JSON::ParserError, ApiUnreachableError nil end - def lfs_authenticate(key, repo) - params = { project: sanitize_path(repo), key_id: key.gsub('key-', '') } + def lfs_authenticate(key_id, repo) + params = { project: sanitize_path(repo), key_id: key_id } resp = post("#{internal_api_endpoint}/lfs_authenticate", params) GitlabLfsAuthentication.build_from_json(resp.body) if resp.code == HTTP_SUCCESS @@ -68,15 +68,14 @@ class GitlabNet get("#{internal_api_endpoint}/check", options: { read_timeout: CHECK_TIMEOUT }) end - def authorized_key(key) - resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}") + def authorized_key(full_key) + resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(full_key, '+/=')}") JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue nil end - def two_factor_recovery_codes(key) - key_id = key.gsub('key-', '') + def two_factor_recovery_codes(key_id) resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id) JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue @@ -92,8 +91,8 @@ class GitlabNet false end - def post_receive(gl_repository, identifier, changes) - params = { gl_repository: gl_repository, identifier: identifier, changes: changes } + def post_receive(gl_repository, actor, changes) + params = { gl_repository: gl_repository, identifier: actor.identifier, changes: changes } resp = post("#{internal_api_endpoint}/post_receive", params) raise NotFoundError if resp.code == HTTP_NOT_FOUND @@ -113,7 +112,7 @@ class GitlabNet repo.delete("'") end - def determine_action(key_id, resp) + def determine_action(actor, resp) json = JSON.parse(resp.body) message = json['message'] @@ -124,7 +123,7 @@ class GitlabNet # accessing the 'status' key. raise AccessDeniedError, message unless json['status'] - Action::Gitaly.create_from_json(key_id, json) + Action::Gitaly.create_from_json(actor, json) when HTTP_UNAUTHORIZED, HTTP_NOT_FOUND raise AccessDeniedError, message else diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index cb9931d..9248582 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -8,20 +8,18 @@ require 'securerandom' class GitlabPostReceive include NamesHelper - attr_reader :config, :gl_repository, :repo_path, :changes, :jid - - def initialize(gl_repository, repo_path, actor, changes) + def initialize(gl_repository, repo_path, gl_id, changes) @config = GitlabConfig.new @gl_repository = gl_repository @repo_path = repo_path.strip - @actor = actor + @gl_id = gl_id @changes = changes @jid = SecureRandom.hex(12) end def exec response = GitlabMetrics.measure("post-receive") do - api.post_receive(gl_repository, @actor, changes) + api.post_receive(gl_repository, actor, changes) end return false unless response @@ -35,12 +33,18 @@ class GitlabPostReceive false end - protected + private + + attr_reader :config, :gl_repository, :repo_path, :gl_id, :changes, :jid def api @api ||= GitlabNet.new end + def actor + @actor ||= Actor.new_from(gl_id, audit_usernames: config.audit_usernames) + end + def print_merge_request_links(merge_request_urls) return if merge_request_urls.empty? puts @@ -100,8 +104,6 @@ class GitlabPostReceive puts "=" * total_width end - private - def parse_broadcast_msg(msg, text_length) msg ||= "" # just return msg if shorter than or equal to text length diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 5c51bc1..6fde88a 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -3,7 +3,7 @@ require 'pathname' require_relative 'gitlab_net' require_relative 'gitlab_metrics' -require_relative 'user' +require_relative 'actor/key' class GitlabShell API_2FA_RECOVERY_CODES_COMMAND = '2fa_recovery_codes'.freeze @@ -16,8 +16,8 @@ class GitlabShell GIT_COMMANDS = [GIT_UPLOAD_PACK_COMMAND, GIT_RECEIVE_PACK_COMMAND, GIT_UPLOAD_ARCHIVE_COMMAND, GIT_LFS_AUTHENTICATE_COMMAND].freeze - def initialize(key_id) - @key_id = key_id + def initialize(key_str) + @key_str = key_str @config = GitlabConfig.new end @@ -26,7 +26,7 @@ class GitlabShell # 'evil command'. def exec(origin_cmd) if !origin_cmd || origin_cmd.empty? - puts "Welcome to GitLab, #{user.username}!" + puts "Welcome to GitLab, #{key.username}!" return true end @@ -38,11 +38,11 @@ class GitlabShell $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable" false rescue AccessDeniedError, UnknownError => ex - $logger.warn('Access denied', command: origin_cmd, user: user.log_username) + $logger.warn('Access denied', command: origin_cmd, user: key.log_username) $stderr.puts "GitLab: #{ex.message}" false rescue DisallowedCommandError - $logger.warn('Denied disallowed command', command: origin_cmd, user: user.log_username) + $logger.warn('Denied disallowed command', command: origin_cmd, user: key.log_username) $stderr.puts 'GitLab: Disallowed command' false rescue InvalidRepositoryPathError @@ -52,10 +52,10 @@ class GitlabShell private - attr_reader :config, :key_id + attr_reader :config, :key_str - def user - @user ||= User.new(key_id, audit_usernames: config.audit_usernames) + def key + @key ||= Actor::Key.from(key_str, audit_usernames: config.audit_usernames) end def parse_cmd(cmd) @@ -96,16 +96,16 @@ class GitlabShell end def determine_action(command, git_access_command, repo_name) - return Action::API2FARecovery.new(key_id) if command == API_2FA_RECOVERY_CODES_COMMAND + return Action::API2FARecovery.new(key) if command == API_2FA_RECOVERY_CODES_COMMAND GitlabMetrics.measure('verify-access') do # GitlatNet#check_access will raise exception in the event of a problem initial_action = api.check_access(git_access_command, nil, - repo_name, key_id, '_any') + repo_name, key, '_any') case command when GIT_LFS_AUTHENTICATE_COMMAND - Action::GitLFSAuthenticate.new(key_id, repo_name) + Action::GitLFSAuthenticate.new(key, repo_name) else initial_action end @@ -113,6 +113,6 @@ class GitlabShell end def api - GitlabNet.new + @api ||= GitlabNet.new end end diff --git a/spec/action/api_2fa_recovery.rb_spec.rb b/spec/action/api_2fa_recovery.rb_spec.rb index 1f5219a..ab09ed2 100644 --- a/spec/action/api_2fa_recovery.rb_spec.rb +++ b/spec/action/api_2fa_recovery.rb_spec.rb @@ -2,7 +2,7 @@ require_relative '../spec_helper' require_relative '../../lib/action/api_2fa_recovery' describe Action::API2FARecovery do - let(:key_id) { "key-#{rand(100) + 100}" } + let(:key_id) { '1' } let(:key) { Actor::Key.new(key_id) } let(:username) { 'testuser' } let(:discover_payload) { { 'username' => username } } @@ -14,7 +14,7 @@ describe Action::API2FARecovery do end subject do - described_class.new(key_id) + described_class.new(key) end describe '#execute' do diff --git a/spec/action/git_lfs_authenticate_spec.rb b/spec/action/git_lfs_authenticate_spec.rb index f9a0791..20740db 100644 --- a/spec/action/git_lfs_authenticate_spec.rb +++ b/spec/action/git_lfs_authenticate_spec.rb @@ -2,8 +2,9 @@ require_relative '../spec_helper' require_relative '../../lib/action/git_lfs_authenticate' describe Action::GitLFSAuthenticate do - let(:key_id) { "key-#{rand(100) + 100}" } + let(:key_id) { '1' } let(:repo_name) { 'gitlab-ci.git' } + let(:key) { Actor::Key.new(key_id) } let(:username) { 'testuser' } let(:discover_payload) { { 'username' => username } } let(:api) { double(GitlabNet) } @@ -14,7 +15,7 @@ describe Action::GitLFSAuthenticate do end subject do - described_class.new(key_id, repo_name) + described_class.new(key, repo_name) end describe '#execute' do diff --git a/spec/action/gitaly_spec.rb b/spec/action/gitaly_spec.rb index 9c35b49..61e4e4b 100644 --- a/spec/action/gitaly_spec.rb +++ b/spec/action/gitaly_spec.rb @@ -5,7 +5,9 @@ describe Action::Gitaly do let(:git_trace_log_file_valid) { '/tmp/git_trace_performance.log' } let(:git_trace_log_file_invalid) { "/bleep-bop#{git_trace_log_file_valid}" } let(:git_trace_log_file_relative) { "..#{git_trace_log_file_valid}" } - let(:key_id) { "key-#{rand(100) + 100}" } + let(:key_id) { '1' } + let(:key_str) { 'key-1' } + let(:key) { Actor::Key.new(key_id) } let(:gl_repository) { 'project-1' } let(:gl_username) { 'testuser' } let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') } @@ -34,7 +36,7 @@ describe Action::Gitaly do end subject do - described_class.new(key_id, gl_repository, gl_username, repository_path, gitaly) + described_class.new(key, gl_repository, gl_username, repository_path, gitaly) end describe '#execute' do @@ -45,7 +47,7 @@ describe Action::Gitaly do 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'LANG' => ENV['LANG'], - 'GL_ID' => key_id, + 'GL_ID' => key_str, 'GL_PROTOCOL' => GitlabNet::GL_PROTOCOL, 'GL_REPOSITORY' => gl_repository, 'GL_USERNAME' => gl_username, @@ -63,7 +65,7 @@ describe Action::Gitaly do { 'repository' => gitaly['repository'], 'gl_repository' => gl_repository, - 'gl_id' => key_id, + 'gl_id' => key_str, 'gl_username' => gl_username } end @@ -94,7 +96,7 @@ describe Action::Gitaly do end end - context 'with an relative config.git_trace_log_file' do + context 'with n relative config.git_trace_log_file' do let(:git_trace_log_file) { git_trace_log_file_relative } it 'returns true' do diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 5a65c48..2dd70af 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -7,8 +7,13 @@ describe GitlabNet, vcr: true do let(:base_api_endpoint) { 'http://localhost:3000/api/v4' } let(:internal_api_endpoint) { 'http://localhost:3000/api/v4/internal' } let(:project) { 'gitlab-org/gitlab-test.git' } - let(:key) { 'key-1' } - let(:key2) { 'key-2' } + + let(:key_id1) { '1' } + let(:key1_str) { "key-#{key_id1}" } + let(:key1) { Actor::Key.new(key1_str) } + + let(:user1) { Actor::User.new('user-1') } + let(:secret) { "0a3938d9d95d807e94d937af3a4fbbea\n" } before do @@ -41,7 +46,7 @@ describe GitlabNet, vcr: true do describe '#discover' do it 'should return user has based on key id' do VCR.use_cassette("discover-ok") do - user = gitlab_net.discover(key) + user = gitlab_net.discover(key_id1) expect(user['name']).to eql 'Administrator' expect(user['username']).to eql 'root' end @@ -50,14 +55,14 @@ describe GitlabNet, vcr: true do it 'adds the secret_token to request' do VCR.use_cassette("discover-ok") do allow_any_instance_of(Net::HTTP::Get).to receive(:set_form_data).with(hash_including(secret_token: secret)) - gitlab_net.discover(key) + gitlab_net.discover(key_id1) end end it "raises an exception if the connection fails" do VCR.use_cassette("discover-ok") do allow_any_instance_of(Net::HTTP).to receive(:request).and_raise(StandardError) - expect(gitlab_net.discover(key)).to be_nil + expect(gitlab_net.discover(key_id1)).to be_nil end end end @@ -66,7 +71,7 @@ describe GitlabNet, vcr: true do context 'lfs authentication succeeded' do it 'should return the correct data' do VCR.use_cassette('lfs-authenticate-ok') do - lfs_access = gitlab_net.lfs_authenticate(key, project) + lfs_access = gitlab_net.lfs_authenticate(key_id1, project) expect(lfs_access.username).to eql 'root' expect(lfs_access.lfs_token).to eql 'Hyzhyde_wLUeyUQsR3tHGTG8eNocVQm4ssioTEsBSdb6KwCSzQ' expect(lfs_access.repository_http_path).to eql URI.join(internal_api_endpoint.sub('api/v4', ''), project).to_s @@ -156,7 +161,7 @@ describe GitlabNet, vcr: true do let(:gl_repository) { "project-1" } let(:changes) { "123456 789012 refs/heads/test\n654321 210987 refs/tags/tag" } let(:params) do - { gl_repository: gl_repository, identifier: key, changes: changes } + { gl_repository: gl_repository, identifier: key1.identifier, changes: changes } end let(:merge_request_urls) do [{ @@ -166,12 +171,11 @@ describe GitlabNet, vcr: true do }] end - subject { gitlab_net.post_receive(gl_repository, key, changes) } + subject { gitlab_net.post_receive(gl_repository, key1, changes) } it 'sends the correct parameters' do allow_any_instance_of(Net::HTTP::Post).to receive(:set_form_data).with(hash_including(params)) - VCR.use_cassette("post-receive") do subject end @@ -226,7 +230,7 @@ describe GitlabNet, vcr: true do describe '#two_factor_recovery_codes' do it 'returns two factor recovery codes' do VCR.use_cassette('two-factor-recovery-codes') do - result = gitlab_net.two_factor_recovery_codes(key) + result = gitlab_net.two_factor_recovery_codes(key1_str) expect(result['success']).to be_truthy expect(result['recovery_codes']).to eq(['f67c514de60c4953','41278385fc00c1e0']) end @@ -268,7 +272,7 @@ describe GitlabNet, vcr: true do it 'raises an UnknownError exception' do VCR.use_cassette('failed-push') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'ssh') end.to raise_error(UnknownError, 'API is not accessible: An internal server error occurred') end end @@ -278,7 +282,7 @@ describe GitlabNet, vcr: true do it 'raises an UnknownError exception' do VCR.use_cassette('failed-push-unparsable') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'ssh') end.to raise_error(UnknownError, 'API is not accessible') end end @@ -288,7 +292,7 @@ describe GitlabNet, vcr: true do context 'ssh key with access nil, to project' do it 'should allow push access for host' do VCR.use_cassette('allowed-push') do - action = gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh') + action = gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'ssh') expect(action).to be_instance_of(Action::Gitaly) end end @@ -296,13 +300,13 @@ describe GitlabNet, vcr: true do it 'adds the secret_token to the request' do VCR.use_cassette('allowed-pull') do allow_any_instance_of(Net::HTTP::Post).to receive(:set_form_data).with(hash_including(secret_token: secret)) - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'ssh') end end it 'should allow pull access for host' do VCR.use_cassette("allowed-pull") do - action = gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'ssh') + action = gitlab_net.check_access('git-upload-pack', nil, project, key1, changes, 'ssh') expect(action).to be_instance_of(Action::Gitaly) end end @@ -312,13 +316,13 @@ describe GitlabNet, vcr: true do it 'should deny pull access for host' do VCR.use_cassette('ssh-pull-disabled-old') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, key1, changes, 'http') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end VCR.use_cassette('ssh-pull-disabled') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, key1, changes, 'http') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -326,13 +330,13 @@ describe GitlabNet, vcr: true do it 'should deny push access for host' do VCR.use_cassette('ssh-push-disabled-old') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end VCR.use_cassette('ssh-push-disabled') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -342,13 +346,13 @@ describe GitlabNet, vcr: true do it 'should deny pull access for host' do VCR.use_cassette('http-pull-disabled-old') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, key1, changes, 'http') end.to raise_error(AccessDeniedError, 'Pulling over HTTP is not allowed.') end VCR.use_cassette('http-pull-disabled') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, key1, changes, 'http') end.to raise_error(AccessDeniedError, 'Pulling over HTTP is not allowed.') end end @@ -356,13 +360,13 @@ describe GitlabNet, vcr: true do it 'should deny push access for host' do VCR.use_cassette('http-push-disabled-old') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'http') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'http') end.to raise_error(AccessDeniedError, 'Pushing over HTTP is not allowed.') end VCR.use_cassette('http-push-disabled') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'http') + gitlab_net.check_access('git-receive-pack', nil, project, key1, changes, 'http') end.to raise_error(AccessDeniedError, 'Pushing over HTTP is not allowed.') end end @@ -372,13 +376,13 @@ describe GitlabNet, vcr: true do it 'should deny pull access for host' do VCR.use_cassette('ssh-pull-project-denied-old') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key2, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, user1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end VCR.use_cassette('ssh-pull-project-denied') do expect do - gitlab_net.check_access('git-receive-pack', nil, project, key2, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, user1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -386,13 +390,13 @@ describe GitlabNet, vcr: true do it 'should deny push access for host' do VCR.use_cassette('ssh-push-project-denied-old') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, key2, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, user1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end VCR.use_cassette('ssh-push-project-denied') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, key2, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, user1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -400,13 +404,13 @@ describe GitlabNet, vcr: true do it 'should deny push access for host (with user)' do VCR.use_cassette('ssh-push-project-denied-with-user-old') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, 'user-2', changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, user1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end VCR.use_cassette('ssh-push-project-denied-with-user') do expect do - gitlab_net.check_access('git-upload-pack', nil, project, 'user-2', changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, user1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -415,7 +419,7 @@ describe GitlabNet, vcr: true do it "raises an exception if the connection fails" do allow_any_instance_of(Net::HTTP).to receive(:request).and_raise(StandardError) expect { - gitlab_net.check_access('git-upload-pack', nil, project, 'user-1', changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, key1, changes, 'ssh') }.to raise_error(GitlabNet::ApiUnreachableError) end end diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index 704623f..c41cceb 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -5,13 +5,13 @@ require 'gitlab_post_receive' describe GitlabPostReceive do let(:repository_path) { "/home/git/repositories" } let(:repo_name) { 'dzaporozhets/gitlab-ci' } - let(:actor) { 'key-123' } + let(:gl_id) { 'key-123' } let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" } let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") } let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) } let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:gl_repository) { "project-1" } - let(:gitlab_post_receive) { GitlabPostReceive.new(gl_repository, repo_path, actor, wrongly_encoded_changes) } + let(:gitlab_post_receive) { GitlabPostReceive.new(gl_repository, repo_path, gl_id, wrongly_encoded_changes) } let(:broadcast_message) { "test " * 10 + "message " * 10 } let(:enqueued_at) { Time.new(2016, 6, 23, 6, 59) } let(:new_merge_request_urls) do diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index b7c0746..456dfcf 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -12,14 +12,17 @@ describe GitlabShell do subject { described_class.new(key_id) } - let(:key_id) { "key-#{rand(100) + 100}" } + let(:key_id) { '1' } + let(:key) { Actor::Key.new(key_id) } let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') } let(:repo_name) { 'gitlab-ci.git' } let(:repo_path) { File.join(tmp_repos_path, repo_name) } let(:gl_repository) { 'project-1' } let(:gl_username) { 'testuser' } + let(:audit_usernames) { true } let(:api) { double(GitlabNet) } + let(:config) { double(GitlabConfig) } let(:gitaly_action) { Action::Gitaly.new( key_id, @@ -32,6 +35,11 @@ describe GitlabShell do let(:git_lfs_authenticate_action) { Action::GitLFSAuthenticate.new(key_id, repo_name) } before do + allow(GitlabConfig).to receive(:new).and_return(config) + allow(config).to receive(:audit_usernames).and_return(audit_usernames) + + allow(Actor::Key).to receive(:from).with(key_id, audit_usernames: audit_usernames).and_return(key) + allow(GitlabNet).to receive(:new).and_return(api) allow(api).to receive(:discover).with(key_id).and_return('username' => gl_username) end @@ -106,7 +114,7 @@ describe GitlabShell do let(:git_access) { '2fa_recovery_codes' } before do - expect(Action::API2FARecovery).to receive(:new).with(key_id).and_return(api_2fa_recovery_action) + expect(Action::API2FARecovery).to receive(:new).with(key).and_return(api_2fa_recovery_action) end it 'returns true' do @@ -117,7 +125,7 @@ describe GitlabShell do context 'when access to the repo is denied' do before do - expect(api).to receive(:check_access).with('git-upload-pack', nil, repo_name, key_id, '_any').and_raise(AccessDeniedError, 'Sorry, access denied') + expect(api).to receive(:check_access).with('git-upload-pack', nil, repo_name, key, '_any').and_raise(AccessDeniedError, 'Sorry, access denied') end it 'prints a message to stderr and returns false' do @@ -128,7 +136,7 @@ describe GitlabShell do context 'when the API is unavailable' do before do - expect(api).to receive(:check_access).with('git-upload-pack', nil, repo_name, key_id, '_any').and_raise(GitlabNet::ApiUnreachableError) + expect(api).to receive(:check_access).with('git-upload-pack', nil, repo_name, key, '_any').and_raise(GitlabNet::ApiUnreachableError) end it 'prints a message to stderr and returns false' do @@ -139,7 +147,7 @@ describe GitlabShell do context 'when access has been verified OK' do before do - expect(api).to receive(:check_access).with(git_access, nil, repo_name, key_id, '_any').and_return(gitaly_action) + expect(api).to receive(:check_access).with(git_access, nil, repo_name, key, '_any').and_return(gitaly_action) end context 'when origin_cmd is git-upload-pack' do @@ -169,11 +177,10 @@ describe GitlabShell do context 'when origin_cmd is git-lfs-authenticate' do let(:origin_cmd) { 'git-lfs-authenticate' } - # let(:fake_payload) { 'FAKE PAYLOAD' } let(:lfs_access) { double(GitlabLfsAuthentication, authentication_payload: fake_payload)} before do - expect(Action::GitLFSAuthenticate).to receive(:new).with(key_id, repo_name).and_return(git_lfs_authenticate_action) + expect(Action::GitLFSAuthenticate).to receive(:new).with(key, repo_name).and_return(git_lfs_authenticate_action) end context 'upload' do @@ -181,7 +188,6 @@ describe GitlabShell do it 'returns true' do expect(git_lfs_authenticate_action).to receive(:execute).with('git-lfs-authenticate', %w{ git-lfs-authenticate gitlab-ci.git upload }).and_return(true) - # expect($stdout).to receive(:puts).with(fake_payload) expect(subject.exec("#{origin_cmd} #{repo_name} upload")).to be_truthy end end @@ -191,14 +197,12 @@ describe GitlabShell do it 'returns true' do expect(git_lfs_authenticate_action).to receive(:execute).with('git-lfs-authenticate', %w{ git-lfs-authenticate gitlab-ci.git download }).and_return(true) - # expect($stdout).to receive(:puts).with(fake_payload) expect(subject.exec("#{origin_cmd} #{repo_name} download")).to be_truthy end context 'for old git-lfs clients' do it 'returns true' do expect(git_lfs_authenticate_action).to receive(:execute).with('git-lfs-authenticate', %w{ git-lfs-authenticate gitlab-ci.git download long_oid }).and_return(true) - # expect($stdout).to receive(:puts).with(fake_payload) expect(subject.exec("#{origin_cmd} #{repo_name} download long_oid")).to be_truthy end end |