diff options
author | Ash McKenzie <amckenzie@gitlab.com> | 2018-08-01 12:16:42 +1000 |
---|---|---|
committer | Ash McKenzie <amckenzie@gitlab.com> | 2018-08-01 12:47:30 +1000 |
commit | 2bdf08e732ad5d959bfebd222e58a7cd4a4971eb (patch) | |
tree | 1676c34376205ace5088b34c4a124c86ca7f8d9e | |
parent | a686b9a0ee4c180b272b26e45c9a2c6cb84c742c (diff) | |
parent | e3fead94b6f71d3501d586cbb2295ea0d1da2b31 (diff) | |
download | gitlab-shell-2bdf08e732ad5d959bfebd222e58a7cd4a4971eb.tar.gz |
Merge remote-tracking branch 'origin/master' into ash.mckenzie/srp-refactor
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | VERSION | 2 | ||||
-rwxr-xr-x | bin/gitlab-shell | 6 | ||||
-rwxr-xr-x | bin/gitlab-shell-authorized-principals-check | 36 | ||||
-rw-r--r-- | lib/action/api_2fa_recovery.rb | 12 | ||||
-rw-r--r-- | lib/action/git_lfs_authenticate.rb | 10 | ||||
-rw-r--r-- | lib/actor.rb | 3 | ||||
-rw-r--r-- | lib/actor/base.rb | 4 | ||||
-rw-r--r-- | lib/actor/key.rb | 2 | ||||
-rw-r--r-- | lib/actor/username.rb | 19 | ||||
-rw-r--r-- | lib/gitlab_keys.rb | 26 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 17 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 26 | ||||
-rw-r--r-- | spec/action/api_2fa_recovery.rb_spec.rb | 8 | ||||
-rw-r--r-- | spec/action/git_lfs_authenticate_spec.rb | 10 | ||||
-rw-r--r-- | spec/actor/key_spec.rb | 2 | ||||
-rw-r--r-- | spec/gitlab_keys_spec.rb | 34 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 64 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 30 |
19 files changed, 211 insertions, 103 deletions
@@ -1,3 +1,6 @@ +v8.0.0 + - SSH certificate support (!207) + v7.2.0 - Update gitaly-proto to 0.109.0 (!216) @@ -1 +1 @@ -7.2.0 +8.0.0 diff --git a/bin/gitlab-shell b/bin/gitlab-shell index 818a328..1016570 100755 --- a/bin/gitlab-shell +++ b/bin/gitlab-shell @@ -5,19 +5,19 @@ unless ENV['SSH_CONNECTION'] exit end -key_str = /key-[0-9]+/.match(ARGV.join).to_s original_cmd = ENV.delete('SSH_ORIGINAL_COMMAND') require_relative '../lib/gitlab_init' # # -# GitLab shell, invoked from ~/.ssh/authorized_keys +# GitLab shell, invoked from ~/.ssh/authorized_keys or from an +# AuthorizedPrincipalsCommand in the key-less SSH CERT mode. # # require File.join(ROOT_PATH, 'lib', 'gitlab_shell') -if GitlabShell.new(key_str).exec(original_cmd) +if GitlabShell.new(ARGV.join).exec(original_cmd) exit 0 else exit 1 diff --git a/bin/gitlab-shell-authorized-principals-check b/bin/gitlab-shell-authorized-principals-check new file mode 100755 index 0000000..25ee612 --- /dev/null +++ b/bin/gitlab-shell-authorized-principals-check @@ -0,0 +1,36 @@ +#!/usr/bin/env ruby + +# +# GitLab shell authorized principals helper. Emits the same sort of +# command="..." line as gitlab-shell-authorized-principals-check, with +# the right options. +# +# Ex. +# bin/gitlab-shell-authorized-keys-check <key-id> <principal1> [<principal2>...] +# +# Returns one line per principal passed in, e.g.: +# command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL} +# [command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL2}] +# +# Expects to be called by the SSH daemon, via configuration like: +# AuthorizedPrincipalsCommandUser root +# AuthorizedPrincipalsCommand /bin/gitlab-shell-authorized-principals-check git %i sshUsers + +abort "# Wrong number of arguments. #{ARGV.size}. Usage: +# gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]" unless ARGV.size >= 2 + +key_id = ARGV[0] +abort '# No key_id provided' if key_id.nil? || key_id == '' + +principals = ARGV[1..-1] +principals.each { |principal| + abort '# An invalid principal was provided' if principal.nil? || principal == '' +} + +require_relative '../lib/gitlab_init' +require_relative '../lib/gitlab_net' +require_relative '../lib/gitlab_keys' + +principals.each { |principal| + puts GitlabKeys.principal_line("username-#{key_id}", principal.dup) +} diff --git a/lib/action/api_2fa_recovery.rb b/lib/action/api_2fa_recovery.rb index 5597ff0..ad8130f 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) - @key = key + def initialize(actor) + @actor = actor end def execute(_, _) @@ -13,7 +13,7 @@ module Action private - attr_reader :key + attr_reader :actor def continue?(question) puts "#{question} (yes/no)" @@ -34,10 +34,10 @@ module Action return end - resp = api.two_factor_recovery_codes(key.key_id) + resp = api.two_factor_recovery_codes(self) if resp['success'] codes = resp['recovery_codes'].join("\n") - $logger.info('API 2FA recovery success', user: key.log_username) + $logger.info('API 2FA recovery success', user: actor.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: key.log_username) + $logger.info('API 2FA recovery error', user: actor.log_username) puts "An error occurred while trying to generate new recovery codes.\n" \ "#{resp['message']}" end diff --git a/lib/action/git_lfs_authenticate.rb b/lib/action/git_lfs_authenticate.rb index 218c71e..d2e6d76 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, repo_name) - @key = key + def initialize(actor, repo_name) + @actor = actor @repo_name = repo_name end def execute(_, _) GitlabMetrics.measure('lfs-authenticate') do - $logger.info('Processing LFS authentication', user: key.log_username) - lfs_access = api.lfs_authenticate(key.key_id, repo_name) + $logger.info('Processing LFS authentication', user: actor.log_username) + lfs_access = api.lfs_authenticate(self, repo_name) return unless lfs_access puts lfs_access.authentication_payload @@ -21,6 +21,6 @@ module Action private - attr_reader :key, :repo_name + attr_reader :actor, :repo_name end end diff --git a/lib/actor.rb b/lib/actor.rb index eab1bc4..4e8b3b8 100644 --- a/lib/actor.rb +++ b/lib/actor.rb @@ -1,6 +1,7 @@ require_relative 'actor/base' require_relative 'actor/key' require_relative 'actor/user' +require_relative 'actor/username' module Actor class UnsupportedActorError < StandardError; end @@ -11,6 +12,8 @@ module Actor Key.from(str, audit_usernames: audit_usernames) when User.id_regex User.from(str, audit_usernames: audit_usernames) + when Username.id_regex + Username.from(str, audit_usernames: audit_usernames) else raise UnsupportedActorError end diff --git a/lib/actor/base.rb b/lib/actor/base.rb index 76cc522..319873f 100644 --- a/lib/actor/base.rb +++ b/lib/actor/base.rb @@ -31,6 +31,10 @@ module Actor "#{self.class.identifier_prefix}-#{id}" end + def identifier_key + self.class.identifier_key + end + def log_username audit_usernames? ? username : "#{klass_name.downcase} with identifier #{identifier}" end diff --git a/lib/actor/key.rb b/lib/actor/key.rb index 411ef15..46f013a 100644 --- a/lib/actor/key.rb +++ b/lib/actor/key.rb @@ -21,7 +21,7 @@ module Actor def username @username ||= begin - user = GitlabNet.new.discover(key_id) + user = GitlabNet.new.discover(self) user ? "@#{user['username']}" : ANONYMOUS_USER end end diff --git a/lib/actor/username.rb b/lib/actor/username.rb new file mode 100644 index 0000000..e0a07dd --- /dev/null +++ b/lib/actor/username.rb @@ -0,0 +1,19 @@ +require_relative 'base' + +module Actor + class Username < Base + alias username identifier + + def self.identifier_prefix + 'username'.freeze + end + + def self.identifier_key + 'username'.freeze + end + + def self.id_regex + /\Ausername\-\d+\Z/ + end + end +end diff --git a/lib/gitlab_keys.rb b/lib/gitlab_keys.rb index 30444c3..3ee2882 100644 --- a/lib/gitlab_keys.rb +++ b/lib/gitlab_keys.rb @@ -9,12 +9,20 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength attr_accessor :auth_file, :key - def self.command(key_id) + def self.command(whatever) + "#{ROOT_PATH}/bin/gitlab-shell #{whatever}" + end + + def self.command_key(key_id) unless /\A[a-z0-9-]+\z/ =~ key_id raise KeyError, "Invalid key_id: #{key_id.inspect}" end - "#{ROOT_PATH}/bin/gitlab-shell #{key_id}" + command(key_id) + end + + def self.whatever_line(command, trailer) + "command=\"#{command}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{trailer}" end def self.key_line(key_id, public_key) @@ -24,7 +32,17 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength raise KeyError, "Invalid public_key: #{public_key.inspect}" end - "command=\"#{command(key_id)}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{public_key}" + whatever_line(command_key(key_id), public_key) + end + + def self.principal_line(username_key_id, principal) + principal.chomp! + + if principal.include?("\n") + raise KeyError, "Invalid principal: #{principal.inspect}" + end + + whatever_line(command_key(username_key_id), principal) end def initialize @@ -119,7 +137,7 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength $logger.info('Removing key', key_id: @key_id) open_auth_file('r+') do |f| while line = f.gets # rubocop:disable Style/AssignmentInCondition - next unless line.start_with?("command=\"#{self.class.command(@key_id)}\"") + 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. diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 34e2ff6..0a3e383 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -26,22 +26,24 @@ class GitlabNet env: env } - params[actor.class.identifier_key.to_sym] = actor.id + params[actor.identifier_key.to_sym] = actor.id resp = post("#{internal_api_endpoint}/allowed", params) determine_action(actor, resp) end - def discover(key_id) - resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}") + def discover(actor) + resp = get("#{internal_api_endpoint}/discover?#{actor.identifier_key}=#{actor.id}") JSON.parse(resp.body) rescue JSON::ParserError, ApiUnreachableError nil end - def lfs_authenticate(key_id, repo) - params = { project: sanitize_path(repo), key_id: key_id } + def lfs_authenticate(actor, repo) + params = { project: sanitize_path(repo) } + params[actor.identifier_key.to_sym] = actor.id + resp = post("#{internal_api_endpoint}/lfs_authenticate", params) GitlabLfsAuthentication.build_from_json(resp.body) if resp.code == HTTP_SUCCESS @@ -75,8 +77,9 @@ class GitlabNet nil end - def two_factor_recovery_codes(key_id) - resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id) + def two_factor_recovery_codes(actor) + params = { actor.identifier_key.to_sym => actor.id } + resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", params) JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue {} diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 68d39bc..7b2b2b1 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 'actor/key' +require_relative 'actor' class GitlabShell API_2FA_RECOVERY_CODES_COMMAND = '2fa_recovery_codes'.freeze @@ -18,9 +18,9 @@ class GitlabShell Struct.new('ParsedCommand', :command, :git_access_command, :repo_name, :args) - def initialize(key_str) - @key_str = key_str + def initialize(who) @config = GitlabConfig.new + @actor = Actor.new_from(who, audit_usernames: @config.audit_usernames) end # The origin_cmd variable contains UNTRUSTED input. If the user ran @@ -28,22 +28,22 @@ class GitlabShell # 'evil command'. def exec(origin_cmd) if !origin_cmd || origin_cmd.empty? - puts "Welcome to GitLab, #{key.username}!" + puts "Welcome to GitLab, #{actor.username}!" return true end parsed_command = parse_cmd(origin_cmd) - action = determine_action(parsed_command) + action = determine_action(parsed_command) # FIXME: watch out action.execute(parsed_command.command, parsed_command.args) rescue GitlabNet::ApiUnreachableError $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: key.log_username) + $logger.warn('Access denied', command: origin_cmd, user: actor.log_username) $stderr.puts "GitLab: #{ex.message}" false rescue DisallowedCommandError - $logger.warn('Denied disallowed command', command: origin_cmd, user: key.log_username) + $logger.warn('Denied disallowed command', command: origin_cmd, user: actor.log_username) $stderr.puts 'GitLab: Disallowed command' false rescue InvalidRepositoryPathError @@ -53,11 +53,7 @@ class GitlabShell private - attr_reader :config, :key_str - - def key - @key ||= Actor::Key.from(key_str, audit_usernames: config.audit_usernames) - end + attr_reader :config, :actor def parse_cmd(cmd) args = Shellwords.shellwords(cmd) @@ -99,7 +95,7 @@ class GitlabShell end def determine_action(parsed_command) - return Action::API2FARecovery.new(key) if parsed_command.command == API_2FA_RECOVERY_CODES_COMMAND + return Action::API2FARecovery.new(actor) if parsed_command.command == API_2FA_RECOVERY_CODES_COMMAND GitlabMetrics.measure('verify-access') do # GitlatNet#check_access will raise exception in the event of a problem @@ -107,13 +103,13 @@ class GitlabShell parsed_command.git_access_command, nil, parsed_command.repo_name, - key, + actor, '_any' ) case parsed_command.command when GIT_LFS_AUTHENTICATE_COMMAND - Action::GitLFSAuthenticate.new(key, parsed_command.repo_name) + Action::GitLFSAuthenticate.new(actor, parsed_command.repo_name) else initial_action end diff --git a/spec/action/api_2fa_recovery.rb_spec.rb b/spec/action/api_2fa_recovery.rb_spec.rb index ab09ed2..bfdadd6 100644 --- a/spec/action/api_2fa_recovery.rb_spec.rb +++ b/spec/action/api_2fa_recovery.rb_spec.rb @@ -3,18 +3,18 @@ require_relative '../../lib/action/api_2fa_recovery' describe Action::API2FARecovery do let(:key_id) { '1' } - let(:key) { Actor::Key.new(key_id) } + let(:actor) { Actor::Key.new(key_id) } let(:username) { 'testuser' } let(:discover_payload) { { 'username' => username } } let(:api) { double(GitlabNet) } before do allow(GitlabNet).to receive(:new).and_return(api) - allow(api).to receive(:discover).with(key_id).and_return(discover_payload) + allow(api).to receive(:discover).with(actor).and_return(discover_payload) end subject do - described_class.new(key) + described_class.new(actor) end describe '#execute' do @@ -46,7 +46,7 @@ describe Action::API2FARecovery do before do expect(subject).to receive(:continue?).and_return(true) - expect(api).to receive(:two_factor_recovery_codes).with(key_id).and_return(response) + expect(api).to receive(:two_factor_recovery_codes).with(subject).and_return(response) end context 'with a unsuccessful response' do diff --git a/spec/action/git_lfs_authenticate_spec.rb b/spec/action/git_lfs_authenticate_spec.rb index 20740db..3c84d0c 100644 --- a/spec/action/git_lfs_authenticate_spec.rb +++ b/spec/action/git_lfs_authenticate_spec.rb @@ -4,24 +4,24 @@ require_relative '../../lib/action/git_lfs_authenticate' describe Action::GitLFSAuthenticate do let(:key_id) { '1' } let(:repo_name) { 'gitlab-ci.git' } - let(:key) { Actor::Key.new(key_id) } + let(:actor) { Actor::Key.new(key_id) } let(:username) { 'testuser' } let(:discover_payload) { { 'username' => username } } let(:api) { double(GitlabNet) } before do allow(GitlabNet).to receive(:new).and_return(api) - allow(api).to receive(:discover).with(key_id).and_return(discover_payload) + allow(api).to receive(:discover).with(actor).and_return(discover_payload) end subject do - described_class.new(key, repo_name) + described_class.new(actor, repo_name) end describe '#execute' do context 'when response from API is not a success' do before do - expect(api).to receive(:lfs_authenticate).with(key_id, repo_name).and_return(nil) + expect(api).to receive(:lfs_authenticate).with(subject, repo_name).and_return(nil) end it 'returns nil' do @@ -36,7 +36,7 @@ describe Action::GitLFSAuthenticate do let(:gitlab_lfs_authentication) { GitlabLfsAuthentication.new(username, lfs_token, repository_http_path) } before do - expect(api).to receive(:lfs_authenticate).with(key_id, repo_name).and_return(gitlab_lfs_authentication) + expect(api).to receive(:lfs_authenticate).with(subject, repo_name).and_return(gitlab_lfs_authentication) end it 'puts payload to stdout' do diff --git a/spec/actor/key_spec.rb b/spec/actor/key_spec.rb index 3c13f76..e61a083 100644 --- a/spec/actor/key_spec.rb +++ b/spec/actor/key_spec.rb @@ -11,7 +11,7 @@ describe Actor::Key do before do allow(GitlabNet).to receive(:new).and_return(api) - allow(api).to receive(:discover).with(key_id).and_return(discover_payload) + allow(api).to receive(:discover).with(subject).and_return(discover_payload) end describe '.from' do diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb index 0baeed7..3cd6798 100644 --- a/spec/gitlab_keys_spec.rb +++ b/spec/gitlab_keys_spec.rb @@ -8,13 +8,25 @@ describe GitlabKeys do end describe '.command' do + it 'the internal "command" utility function' do + command = "#{ROOT_PATH}/bin/gitlab-shell does-not-validate" + expect(described_class.command('does-not-validate')).to eq(command) + end + + it 'does not raise a KeyError on invalid input' do + command = "#{ROOT_PATH}/bin/gitlab-shell foo\nbar\nbaz\n" + expect(described_class.command("foo\nbar\nbaz\n")).to eq(command) + end + end + + describe '.command_key' do it 'returns the "command" part of the key line' do command = "#{ROOT_PATH}/bin/gitlab-shell key-123" - expect(described_class.command('key-123')).to eq(command) + expect(described_class.command_key('key-123')).to eq(command) end it 'raises KeyError on invalid input' do - expect do described_class.command("\nssh-rsa AAA") end.to raise_error(described_class::KeyError) + expect { described_class.command_key("\nssh-rsa AAA") }.to raise_error(described_class::KeyError) end end @@ -30,7 +42,23 @@ describe GitlabKeys do end it 'raises KeyError on invalid input' do - expect do described_class.key_line('key-741', "ssh-rsa AAA\nssh-rsa AAA") end.to raise_error(described_class::KeyError) + expect { described_class.key_line('key-741', "ssh-rsa AAA\nssh-rsa AAA") }.to raise_error(described_class::KeyError) + end + end + + describe '.principal_line' do + let(:line) { %(command="#{ROOT_PATH}/bin/gitlab-shell username-someuser",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty sshUsers) } + + it 'returns the key line' do + expect(described_class.principal_line('username-someuser', 'sshUsers')).to eq(line) + end + + it 'silently removes a trailing newline' do + expect(described_class.principal_line('username-someuser', "sshUsers\n")).to eq(line) + end + + it 'raises KeyError on invalid input' do + expect { described_class.principal_line('username-someuser', "sshUsers\nloginUsers") }.to raise_error(described_class::KeyError) end end diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 2dd70af..a02744f 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -8,11 +8,9 @@ describe GitlabNet, vcr: true do let(:internal_api_endpoint) { 'http://localhost:3000/api/v4/internal' } let(:project) { 'gitlab-org/gitlab-test.git' } - 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(:actor1) { Actor.new_from('key-1') } + let(:bad_actor1) { Actor.new_from('key-777') } + let(:actor2) { Actor.new_from('user-1') } let(:secret) { "0a3938d9d95d807e94d937af3a4fbbea\n" } @@ -46,7 +44,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_id1) + user = gitlab_net.discover(actor1) expect(user['name']).to eql 'Administrator' expect(user['username']).to eql 'root' end @@ -55,14 +53,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_id1) + gitlab_net.discover(actor1) 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_id1)).to be_nil + expect(gitlab_net.discover(actor1)).to be_nil end end end @@ -71,7 +69,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_id1, project) + lfs_access = gitlab_net.lfs_authenticate(actor1, 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 @@ -161,7 +159,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: key1.identifier, changes: changes } + { gl_repository: gl_repository, identifier: actor1.identifier, changes: changes } end let(:merge_request_urls) do [{ @@ -171,7 +169,7 @@ describe GitlabNet, vcr: true do }] end - subject { gitlab_net.post_receive(gl_repository, key1, changes) } + subject { gitlab_net.post_receive(gl_repository, actor1, changes) } it 'sends the correct parameters' do allow_any_instance_of(Net::HTTP::Post).to receive(:set_form_data).with(hash_including(params)) @@ -230,7 +228,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(key1_str) + result = gitlab_net.two_factor_recovery_codes(actor1) expect(result['success']).to be_truthy expect(result['recovery_codes']).to eq(['f67c514de60c4953','41278385fc00c1e0']) end @@ -238,7 +236,7 @@ describe GitlabNet, vcr: true do it 'returns false when recovery codes cannot be generated' do VCR.use_cassette('two-factor-recovery-codes-fail') do - result = gitlab_net.two_factor_recovery_codes('key-777') + result = gitlab_net.two_factor_recovery_codes(bad_actor1) expect(result['success']).to be_falsey expect(result['message']).to eq('Could not find the given key') end @@ -272,7 +270,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, key1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, changes, 'ssh') end.to raise_error(UnknownError, 'API is not accessible: An internal server error occurred') end end @@ -282,7 +280,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, key1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, changes, 'ssh') end.to raise_error(UnknownError, 'API is not accessible') end end @@ -292,7 +290,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, key1, changes, 'ssh') + action = gitlab_net.check_access('git-receive-pack', nil, project, actor1, changes, 'ssh') expect(action).to be_instance_of(Action::Gitaly) end end @@ -300,13 +298,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, key1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, 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, key1, changes, 'ssh') + action = gitlab_net.check_access('git-upload-pack', nil, project, actor1, changes, 'ssh') expect(action).to be_instance_of(Action::Gitaly) end end @@ -316,13 +314,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, key1, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, actor1, 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, key1, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, actor1, changes, 'http') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -330,13 +328,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, key1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, 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, key1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -346,13 +344,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, key1, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, actor1, 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, key1, changes, 'http') + gitlab_net.check_access('git-upload-pack', nil, project, actor1, changes, 'http') end.to raise_error(AccessDeniedError, 'Pulling over HTTP is not allowed.') end end @@ -360,13 +358,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, key1, changes, 'http') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, 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, key1, changes, 'http') + gitlab_net.check_access('git-receive-pack', nil, project, actor1, changes, 'http') end.to raise_error(AccessDeniedError, 'Pushing over HTTP is not allowed.') end end @@ -376,13 +374,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, user1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor2, 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, user1, changes, 'ssh') + gitlab_net.check_access('git-receive-pack', nil, project, actor2, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -390,13 +388,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, user1, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, actor2, 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, user1, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, actor2, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -404,13 +402,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, user1, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, actor2, 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, user1, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, actor2, changes, 'ssh') end.to raise_error(AccessDeniedError, 'Git access over SSH is not allowed') end end @@ -419,7 +417,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, key1, changes, 'ssh') + gitlab_net.check_access('git-upload-pack', nil, project, actor1, changes, 'ssh') }.to raise_error(GitlabNet::ApiUnreachableError) end end diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 456dfcf..201bc62 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -10,44 +10,44 @@ describe GitlabShell do after { FileUtils.rm_rf(tmp_repos_path) } - subject { described_class.new(key_id) } + subject { described_class.new(who) } - let(:key_id) { '1' } - let(:key) { Actor::Key.new(key_id) } + let(:who) { 'key-1' } + let(:audit_usernames) { true } + let(:actor) { Actor.new_from(who, audit_usernames: audit_usernames) } 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, + actor, gl_repository, gl_username, repo_path, { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default' } , 'address' => 'unix:gitaly.socket' }) } - let(:api_2fa_recovery_action) { Action::API2FARecovery.new(key_id) } - let(:git_lfs_authenticate_action) { Action::GitLFSAuthenticate.new(key_id, repo_name) } + let(:api_2fa_recovery_action) { Action::API2FARecovery.new(actor) } + let(:git_lfs_authenticate_action) { Action::GitLFSAuthenticate.new(actor, 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(Actor).to receive(:new_from).with(who, audit_usernames: audit_usernames).and_return(actor) allow(GitlabNet).to receive(:new).and_return(api) - allow(api).to receive(:discover).with(key_id).and_return('username' => gl_username) + allow(api).to receive(:discover).with(actor).and_return('username' => gl_username) end describe '#exec' do context "when we don't have a valid user" do before do - allow(api).to receive(:discover).with(key_id).and_return(nil) + allow(api).to receive(:discover).with(actor).and_return(nil) end it 'prints Welcome.. and returns true' do @@ -114,7 +114,7 @@ describe GitlabShell do let(:git_access) { '2fa_recovery_codes' } before do - expect(Action::API2FARecovery).to receive(:new).with(key).and_return(api_2fa_recovery_action) + expect(Action::API2FARecovery).to receive(:new).with(actor).and_return(api_2fa_recovery_action) end it 'returns true' do @@ -125,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, '_any').and_raise(AccessDeniedError, 'Sorry, access denied') + expect(api).to receive(:check_access).with('git-upload-pack', nil, repo_name, actor, '_any').and_raise(AccessDeniedError, 'Sorry, access denied') end it 'prints a message to stderr and returns false' do @@ -136,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, '_any').and_raise(GitlabNet::ApiUnreachableError) + expect(api).to receive(:check_access).with('git-upload-pack', nil, repo_name, actor, '_any').and_raise(GitlabNet::ApiUnreachableError) end it 'prints a message to stderr and returns false' do @@ -147,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, '_any').and_return(gitaly_action) + expect(api).to receive(:check_access).with(git_access, nil, repo_name, actor, '_any').and_return(gitaly_action) end context 'when origin_cmd is git-upload-pack' do @@ -180,7 +180,7 @@ describe GitlabShell do let(:lfs_access) { double(GitlabLfsAuthentication, authentication_payload: fake_payload)} before do - expect(Action::GitLFSAuthenticate).to receive(:new).with(key, repo_name).and_return(git_lfs_authenticate_action) + expect(Action::GitLFSAuthenticate).to receive(:new).with(actor, repo_name).and_return(git_lfs_authenticate_action) end context 'upload' do |