Revert "Merge branch 'ash.mckenzie/srp-refactor' into 'master'"

This reverts commit 3aaf4751, reversing
changes made to c6577e0d.

Gemfile

 source "http://rubygems.org"
 
 group :development, :test do
-  gem 'guard-rspec', '~> 4.0'
-  gem 'listen', '~> 3.0.0'
-  gem 'rspec', '~> 3.0'
+  gem 'guard', '~> 1.5.0'
+  gem 'guard-rspec', '~> 2.1.0'
+  gem 'listen', '~> 0.5.0'
+  gem 'rspec', '~> 2.0'
   gem 'rspec-its', '~> 1.0.0'
   gem 'rubocop', '0.49.1', require: false
   gem 'simplecov', '~> 0.9.0', require: false
-  gem 'vcr', '~> 4.0'
+  gem 'vcr', '~> 2.4.0'
   gem 'webmock', '~> 1.9.0'
 end

Gemfile.lock

@@ -9,32 +9,18 @@ GEM
       safe_yaml (~> 1.0.0)
     diff-lcs (1.3)
     docile (1.1.5)
-    ffi (1.9.25)
-    formatador (0.2.5)
-    guard (2.14.2)
-      formatador (>= 0.2.4)
-      listen (>= 2.7, < 4.0)
-      lumberjack (>= 1.0.12, < 2.0)
-      nenv (~> 0.1)
-      notiffany (~> 0.0)
-      pry (>= 0.9.12)
-      shellany (~> 0.0)
-      thor (>= 0.18.1)
-    guard-compat (1.2.1)
-    guard-rspec (4.7.3)
-      guard (~> 2.1)
-      guard-compat (~> 1.1)
-      rspec (>= 2.99.0, < 4.0)
-    listen (3.0.8)
-      rb-fsevent (~> 0.9, >= 0.9.4)
-      rb-inotify (~> 0.9, >= 0.9.7)
+    guard (1.5.4)
+      listen (>= 0.4.2)
+      lumberjack (>= 1.0.2)
+      pry (>= 0.9.10)
+      thor (>= 0.14.6)
+    guard-rspec (2.1.2)
+      guard (>= 1.1)
+      rspec (~> 2.11)
+    listen (0.5.3)
     lumberjack (1.0.13)
     method_source (0.9.0)
     multi_json (1.13.1)
-    nenv (0.3.0)
-    notiffany (0.1.1)
-      nenv (~> 0.1)
-      shellany (~> 0.0)
     parallel (1.12.1)
     parser (2.5.1.2)
       ast (~> 2.4.0)
@@ -46,25 +32,17 @@ GEM
     rainbow (2.2.2)
       rake
     rake (12.3.1)
-    rb-fsevent (0.10.3)
-    rb-inotify (0.9.10)
-      ffi (>= 0.5.0, < 2)
-    rspec (3.7.0)
-      rspec-core (~> 3.7.0)
-      rspec-expectations (~> 3.7.0)
-      rspec-mocks (~> 3.7.0)
-    rspec-core (3.7.1)
-      rspec-support (~> 3.7.0)
-    rspec-expectations (3.7.0)
-      diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.7.0)
+    rspec (2.99.0)
+      rspec-core (~> 2.99.0)
+      rspec-expectations (~> 2.99.0)
+      rspec-mocks (~> 2.99.0)
+    rspec-core (2.99.2)
+    rspec-expectations (2.99.2)
+      diff-lcs (>= 1.1.3, < 2.0)
     rspec-its (1.0.1)
       rspec-core (>= 2.99.0.beta1)
       rspec-expectations (>= 2.99.0.beta1)
-    rspec-mocks (3.7.0)
-      diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.7.0)
-    rspec-support (3.7.1)
+    rspec-mocks (2.99.4)
     rubocop (0.49.1)
       parallel (~> 1.10)
       parser (>= 2.3.3.1, < 3.0)
@@ -74,7 +52,6 @@ GEM
       unicode-display_width (~> 1.0, >= 1.0.1)
     ruby-progressbar (1.9.0)
     safe_yaml (1.0.4)
-    shellany (0.0.1)
     simplecov (0.9.2)
       docile (~> 1.1.0)
       multi_json (~> 1.0)
@@ -82,7 +59,7 @@ GEM
     simplecov-html (0.9.0)
     thor (0.20.0)
     unicode-display_width (1.4.0)
-    vcr (4.0.0)
+    vcr (2.4.0)
     webmock (1.9.3)
       addressable (>= 2.2.7)
       crack (>= 0.3.2)
@@ -91,13 +68,14 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  guard-rspec (~> 4.0)
-  listen (~> 3.0.0)
-  rspec (~> 3.0)
+  guard (~> 1.5.0)
+  guard-rspec (~> 2.1.0)
+  listen (~> 0.5.0)
+  rspec (~> 2.0)
   rspec-its (~> 1.0.0)
   rubocop (= 0.49.1)
   simplecov (~> 0.9.0)
-  vcr (~> 4.0)
+  vcr (~> 2.4.0)
   webmock (~> 1.9.0)
 
 BUNDLED WITH

Guardfile

+# A sample Guardfile
 # More info at https://github.com/guard/guard#readme
 
-guard :rspec, cmd: 'bundle exec rspec' do
+guard 'rspec' do
   watch(%r{^spec/.+_spec\.rb$})
-  watch(%r{^lib/(.+)\.rb$})     { |m| "spec/#{m[1]}_spec.rb" }
+  watch(%r{^lib/(.+)\.rb$})     { |m| "spec/lib/#{m[1]}_spec.rb" }
   watch('spec/spec_helper.rb')  { "spec" }
+
+  # Rails example
+  watch(%r{^app/(.+)\.rb$})                           { |m| "spec/#{m[1]}_spec.rb" }
+  watch(%r{^app/(.*)(\.erb|\.haml)$})                 { |m| "spec/#{m[1]}#{m[2]}_spec.rb" }
+  watch(%r{^app/controllers/(.+)_(controller)\.rb$})  { |m| ["spec/routing/#{m[1]}_routing_spec.rb", "spec/#{m[2]}s/#{m[1]}_#{m[2]}_spec.rb", "spec/acceptance/#{m[1]}_spec.rb"] }
+  watch(%r{^spec/support/(.+)\.rb$})                  { "spec" }
+  watch('config/routes.rb')                           { "spec/routing" }
+  watch('app/controllers/application_controller.rb')  { "spec/controllers" }
+
+  # Capybara features specs
+  watch(%r{^app/views/(.+)/.*\.(erb|haml)$})          { |m| "spec/features/#{m[1]}_spec.rb" }
+
+  # Turnip features and steps
+  watch(%r{^spec/acceptance/(.+)\.feature$})
+  watch(%r{^spec/acceptance/steps/(.+)_steps\.rb$})   { |m| Dir[File.join("**/#{m[1]}.feature")][0] || 'spec/acceptance' }
 end

bin/authorized_keys

@@ -10,16 +10,16 @@
 #   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaC1yc2EAAAADAQA...
 #
 
-full_key = ARGV[0]
-abort "# No key provided" if full_key.nil? || full_key.empty?
+key = ARGV[0]
+abort "# No key provided" if key.nil? || key.empty?
 
 require_relative "../lib/gitlab_init"
 require_relative "../lib/gitlab_net"
 require_relative "../lib/gitlab_keys"
 
-authorized_key = GitlabNet.new.authorized_key(full_key)
+authorized_key = GitlabNet.new.authorized_key(key)
 if authorized_key.nil?
-  puts "# No key was found for #{full_key}"
+  puts "# No key was found for #{key}"
 else
-  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
+  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key["key"])
 end

bin/gitlab-keys

@@ -13,7 +13,7 @@ require_relative '../lib/gitlab_init'
 #   /bin/gitlab-keys rm-key key-23 "ssh-rsa AAAAx321..."
 #
 #   /bin/gitlab-keys list-keys
-#
+#   
 #   /bin/gitlab-keys clear
 #
 

bin/gitlab-shell-authorized-keys-check

@@ -5,7 +5,7 @@
 # command for a given ssh key fingerprint
 #
 # Ex.
-#   bin/gitlab-shell-authorized-keys-check <expected-username> <actual-username> <public-key>
+#   bin/gitlab-shell-authorized-keys-check <username> <public-key>
 #
 # Returns
 #   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA...
@@ -27,16 +27,16 @@ abort '# No username provided' if actual_username.nil? || actual_username == ''
 # Normally, these would both be 'git', but it can be configured by the user
 exit 0 unless expected_username == actual_username
 
-full_key = ARGV[2]
-abort "# No key provided" if full_key.nil? || full_key == ''
+key = ARGV[2]
+abort "# No key provided" if key.nil? || key == ''
 
 require_relative '../lib/gitlab_init'
 require_relative '../lib/gitlab_net'
 require_relative '../lib/gitlab_keys'
 
-authorized_key = GitlabNet.new.authorized_key(full_key)
+authorized_key = GitlabNet.new.authorized_key(key)
 if authorized_key.nil?
-  puts "# No key was found for #{full_key}"
+  puts "# No key was found for #{key}"
 else
   puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
 end

bin/gitlab-shell-authorized-principals-check

@@ -6,7 +6,7 @@
 # the right options.
 #
 # Ex.
-#   bin/gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]
+#   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}
@@ -23,9 +23,9 @@ key_id = ARGV[0]
 abort '# No key_id provided' if key_id.nil? || key_id == ''
 
 principals = ARGV[1..-1]
-principals.each do |principal|
-  abort '# An invalid principal was provided' if principal.nil? || principal == ''
-end
+principals.each { |principal|
+  abort '# An invalid principal was provided' if principal.nil? || principal == ''  
+}
 
 require_relative '../lib/gitlab_init'
 require_relative '../lib/gitlab_net'

hooks/post-receive

@@ -4,15 +4,15 @@
 # will be processed properly.
 
 refs = $stdin.read
-gl_id = ENV.delete('GL_ID')
+key_id = ENV.delete('GL_ID')
 gl_repository = ENV['GL_REPOSITORY']
 repo_path = Dir.pwd
 
 require_relative '../lib/gitlab_custom_hook'
 require_relative '../lib/gitlab_post_receive'
 
-if GitlabPostReceive.new(gl_repository, repo_path, gl_id, refs).exec &&
-    GitlabCustomHook.new(repo_path, gl_id).post_receive(refs)
+if GitlabPostReceive.new(gl_repository, repo_path, key_id, refs).exec &&
+    GitlabCustomHook.new(repo_path, key_id).post_receive(refs)
   exit 0
 else
   exit 1

hooks/pre-receive

@@ -4,7 +4,7 @@
 # will be processed properly.
 
 refs = $stdin.read
-gl_id = ENV.delete('GL_ID')
+key_id = ENV.delete('GL_ID')
 protocol = ENV.delete('GL_PROTOCOL')
 repo_path = Dir.pwd
 gl_repository = ENV['GL_REPOSITORY']
@@ -23,8 +23,8 @@ require_relative '../lib/gitlab_net'
 # last so that it only runs if everything else succeeded. On post-receive on the
 # other hand, we run GitlabPostReceive first because the push is already done
 # and we don't want to skip it if the custom hook fails.
-if GitlabAccess.new(gl_repository, repo_path, gl_id, refs, protocol).exec &&
-    GitlabCustomHook.new(repo_path, gl_id).pre_receive(refs) &&
+if GitlabAccess.new(gl_repository, repo_path, key_id, refs, protocol).exec &&
+    GitlabCustomHook.new(repo_path, key_id).pre_receive(refs) &&
     increase_reference_counter(gl_repository, repo_path)
   exit 0
 else

hooks/update

@@ -7,11 +7,11 @@ ref_name  = ARGV[0]
 old_value = ARGV[1]
 new_value = ARGV[2]
 repo_path = Dir.pwd
-gl_id     = ENV.delete('GL_ID')
+key_id    = ENV.delete('GL_ID')
 
 require_relative '../lib/gitlab_custom_hook'
 
-if GitlabCustomHook.new(repo_path, gl_id).update(ref_name, old_value, new_value)
+if GitlabCustomHook.new(repo_path, key_id).update(ref_name, old_value, new_value)
   exit 0
 else
   exit 1

lib/action.rb

-require_relative 'action/base'
-require_relative 'action/gitaly'
-require_relative 'action/git_lfs_authenticate'
-require_relative 'action/api_2fa_recovery'
-
-module Action
-end

lib/action/api_2fa_recovery.rb

-require_relative '../action'
-require_relative '../gitlab_logger'
-
-module Action
-  class API2FARecovery < Base
-    def initialize(actor)
-      @actor = actor
-    end
-
-    def execute(_, _)
-      recover
-    end
-
-    private
-
-    attr_reader :actor
-
-    def continue?(question)
-      puts "#{question} (yes/no)"
-      STDOUT.flush # Make sure the question gets output before we wait for input
-      response = STDIN.gets.chomp
-      puts '' # Add a buffer in the output
-      response == 'yes'
-    end
-
-    def recover
-      continue = continue?(
-        "Are you sure you want to generate new two-factor recovery codes?\n" \
-        "Any existing recovery codes you saved will be invalidated."
-      )
-
-      unless continue
-        puts 'New recovery codes have *not* been generated. Existing codes will remain valid.'
-        return
-      end
-
-      resp = api.two_factor_recovery_codes(actor)
-      if resp['success']
-        codes = resp['recovery_codes'].join("\n")
-        $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" \
-            "your two-factor code. Then, visit your Profile Settings and add\n" \
-            "a new device so you do not lose access to your account again."
-        true
-      else
-        $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
-    end
-  end
-end

lib/action/base.rb

-require 'json'
-
-require_relative '../gitlab_config'
-require_relative '../gitlab_net'
-require_relative '../gitlab_metrics'
-
-module Action
-  class Base
-    def initialize
-      raise NotImplementedError
-    end
-
-    def self.create_from_json(_)
-      raise NotImplementedError
-    end
-
-    private
-
-    def config
-      @config ||= GitlabConfig.new
-    end
-
-    def api
-      @api ||= GitlabNet.new
-    end
-  end
-end

lib/action/git_lfs_authenticate.rb

-require_relative '../action'
-require_relative '../gitlab_logger'
-
-module Action
-  class GitLFSAuthenticate < Base
-    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: actor.log_username)
-        lfs_access = api.lfs_authenticate(actor, repo_name)
-        return unless lfs_access
-
-        puts lfs_access.authentication_payload
-      end
-      true
-    end
-
-    private
-
-    attr_reader :actor, :repo_name
-  end
-end

lib/action/gitaly.rb

-require_relative '../action'
-require_relative '../gitlab_logger'
-require_relative '../gitlab_net'
-
-module Action
-  class Gitaly < Base
-    REPOSITORY_PATH_NOT_PROVIDED = "Repository path not provided. Please make sure you're using GitLab v8.10 or later.".freeze
-    MIGRATED_COMMANDS = {
-      'git-upload-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-upload-pack'),
-      'git-upload-archive' => File.join(ROOT_PATH, 'bin', 'gitaly-upload-archive'),
-      'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack')
-    }.freeze
-
-    def initialize(actor, gl_repository, gl_username, git_protocol, repository_path, gitaly)
-      @actor = actor
-      @gl_repository = gl_repository
-      @gl_username = gl_username
-      @git_protocol = git_protocol
-      @repository_path = repository_path
-      @gitaly = gitaly
-    end
-
-    def self.create_from_json(actor, json)
-      new(actor,
-          json['gl_repository'],
-          json['gl_username'],
-          json['git_protocol'],
-          json['repository_path'],
-          json['gitaly'])
-    end
-
-    def execute(command, args)
-      raise ArgumentError, REPOSITORY_PATH_NOT_PROVIDED unless repository_path
-      raise InvalidRepositoryPathError unless valid_repository?
-
-      $logger.info('Performing Gitaly command', user: actor.log_username)
-      process(command, args)
-    end
-
-    private
-
-    attr_reader :actor, :gl_repository, :gl_username, :repository_path, :gitaly
-
-    def git_protocol
-      @git_protocol || ENV['GIT_PROTOCOL'] # TODO: tidy this up
-    end
-
-    def process(command, args)
-      executable = command
-      args = [repository_path]
-
-      if MIGRATED_COMMANDS.key?(executable) && gitaly
-        executable = MIGRATED_COMMANDS[executable]
-        gitaly_address = gitaly['address']
-        args = [gitaly_address, JSON.dump(gitaly_request)]
-      end
-
-      args_string = [File.basename(executable), *args].join(' ')
-      $logger.info('executing git command', command: args_string, user: actor.log_username)
-
-      exec_cmd(executable, *args)
-    end
-
-    def exec_cmd(*args)
-      env = exec_env
-      env['GITALY_TOKEN'] = gitaly['token'] if gitaly && gitaly.include?('token')
-
-      if git_trace_available?
-        env.merge!(
-          'GIT_TRACE' => config.git_trace_log_file,
-          'GIT_TRACE_PACKET' => config.git_trace_log_file,
-          'GIT_TRACE_PERFORMANCE' => config.git_trace_log_file
-        )
-      end
-
-      # We use 'chdir: ROOT_PATH' to let the next executable know where config.yml is.
-      Kernel.exec(env, *args, unsetenv_others: true, chdir: ROOT_PATH)
-    end
-
-    def exec_env
-      {
-        'HOME' => ENV['HOME'],
-        'PATH' => ENV['PATH'],
-        'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'],
-        'LANG' => ENV['LANG'],
-        'GL_ID' => actor.identifier,
-        'GL_PROTOCOL' => GitlabNet::GL_PROTOCOL,
-        'GL_REPOSITORY' => gl_repository,
-        'GL_USERNAME' => gl_username
-      }
-    end
-
-    def gitaly_request
-      # The entire gitaly_request hash should be built in gitlab-ce and passed
-      # on as-is. For now we build a fake one on the spot.
-      {
-        'repository' => gitaly['repository'],
-        'gl_repository' => gl_repository,
-        'gl_id' => actor.identifier,
-        'gl_username' => gl_username,
-        'git_protocol' => git_protocol
-      }
-    end
-
-    def valid_repository?
-      File.absolute_path(repository_path) == repository_path
-    end
-
-    def git_trace_available?
-      return false unless config.git_trace_log_file
-
-      if Pathname(config.git_trace_log_file).relative?
-        $logger.warn('git trace log path must be absolute, ignoring', git_trace_log_file: config.git_trace_log_file)
-        return false
-      end
-
-      begin
-        File.open(config.git_trace_log_file, 'a') { nil }
-        return true
-      rescue => ex
-        $logger.warn('Failed to open git trace log file', git_trace_log_file: config.git_trace_log_file, error: ex.to_s)
-        return false
-      end
-    end
-  end
-end

lib/actor.rb

-require_relative 'actor/base'
-require_relative 'actor/key'
-require_relative 'actor/user'
-require_relative 'actor/username'
-
-module Actor
-  class UnsupportedActorError < StandardError; end
-
-  def self.new_from(str, audit_usernames: false)
-    case str
-    when Key.id_regex
-      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
-  end
-end

lib/actor/base.rb

-module Actor
-  class Base
-    attr_reader :id
-
-    def initialize(id, audit_usernames: false)
-      @id = id
-      @audit_usernames = audit_usernames
-    end
-
-    def self.from(str, audit_usernames: false)
-      new(str.gsub(/#{identifier_prefix}-/, ''), audit_usernames: audit_usernames)
-    end
-
-    def self.identifier_key
-      raise NotImplementedError
-    end
-
-    def self.identifier_prefix
-      raise NotImplementedError
-    end
-
-    def self.id_regex
-      raise NotImplementedError
-    end
-
-    def username
-      raise NotImplementedError
-    end
-
-    def identifier
-      "#{self.class.identifier_prefix}-#{id}"
-    end
-
-    def identifier_key
-      self.class.identifier_key
-    end
-
-    def log_username
-      audit_usernames? ? username : "#{label} with identifier #{identifier}"
-    end
-
-    private
-
-    attr_reader :audit_usernames
-
-    alias audit_usernames? audit_usernames
-
-    def klass_name
-      self.class.to_s.split('::')[-1]
-    end
-
-    def label
-      klass_name.downcase
-    end
-  end
-end

lib/actor/key.rb

-require_relative 'base'
-require_relative '../gitlab_net'
-
-module Actor
-  class Key < Base
-    ANONYMOUS_USER = 'Anonymous'.freeze
-
-    alias key_id id
-
-    def self.identifier_prefix
-      'key'.freeze
-    end
-
-    def self.identifier_key
-      'key_id'.freeze
-    end
-
-    def self.id_regex
-      /\Akey\-\d+\Z/
-    end
-
-    def username
-      @username ||= begin
-        user = GitlabNet.new.discover(self)
-        user ? "@#{user['username']}" : ANONYMOUS_USER
-      end
-    end
-  end
-end

lib/actor/user.rb

-require_relative 'base'
-
-module Actor
-  class User < Base
-    alias username identifier
-
-    def self.identifier_prefix
-      'user'.freeze
-    end
-
-    def self.identifier_key
-      'user_id'.freeze
-    end
-
-    def self.id_regex
-      /\Auser\-\d+\Z/
-    end
-  end
-end

lib/actor/username.rb

-require_relative 'base'
-require_relative 'key'
-
-module Actor
-  class Username < Key
-    def self.identifier_prefix
-      'username'.freeze
-    end
-
-    def self.identifier_key
-      'username'.freeze
-    end
-
-    def self.id_regex
-      /\Ausername\-[a-z0-9-]+\z/
-    end
-
-    private
-
-    # Override Base#label
-    def label
-      'user'
-    end
-  end
-end