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

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

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

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

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

lib/gitlab_access.rb

 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

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

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

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

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

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

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

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

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