From 505dc808b3c0dc98413506446d368b91b56ff682 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 8 Aug 2016 12:01:25 +0200
Subject: [PATCH 01/41] Use a permissions of user to access all dependent
 projects from CI jobs (this also includes a container images, and in future
 LFS files)

---
 app/controllers/jwt_controller.rb             | 18 +++++----
 .../projects/git_http_client_controller.rb    | 12 +++++-
 .../projects/git_http_controller.rb           |  2 +-
 app/helpers/lfs_helper.rb                     | 16 +++++++-
 app/models/ci/build.rb                        | 13 +++---
 app/models/project.rb                         |  6 ---
 app/policies/project_policy.rb                | 15 +++++--
 ...ntainer_registry_authentication_service.rb | 40 +++++++++++++++++--
 .../20160808085531_add_token_to_build.rb      | 10 +++++
 ...0160808085602_add_index_for_build_token.rb | 12 ++++++
 lib/ci/api/helpers.rb                         | 14 +++++--
 lib/gitlab/auth.rb                            | 31 ++++++++++----
 lib/gitlab/git_access.rb                      | 19 +++++++--
 13 files changed, 163 insertions(+), 45 deletions(-)
 create mode 100644 db/migrate/20160808085531_add_token_to_build.rb
 create mode 100644 db/migrate/20160808085602_add_index_for_build_token.rb

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 66ebdcc37a7..ca02df28b91 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -11,7 +11,7 @@ class JwtController < ApplicationController
     service = SERVICES[params[:service]]
     return head :not_found unless service
 
-    result = service.new(@project, @user, auth_params).execute
+    result = service.new(@project, @user, auth_params).execute(access_type: @access_type)
 
     render json: result, status: result[:http_status]
   end
@@ -21,10 +21,10 @@ class JwtController < ApplicationController
   def authenticate_project_or_user
     authenticate_with_http_basic do |login, password|
       # if it's possible we first try to authenticate project with login and password
-      @project = authenticate_project(login, password)
+      @project, @user, @access_type = authenticate_build(login, password)
       return if @project
 
-      @user = authenticate_user(login, password)
+      @user, @access_type = authenticate_user(login, password)
       return if @user
 
       render_403
@@ -35,15 +35,17 @@ class JwtController < ApplicationController
     params.permit(:service, :scope, :account, :client_id)
   end
 
-  def authenticate_project(login, password)
-    if login == 'gitlab-ci-token'
-      Project.with_builds_enabled.find_by(runners_token: password)
-    end
+  def authenticate_build(login, password)
+    return unless login == 'gitlab-ci-token'
+    return unless password
+
+    build = Ci::Build.running.find_by(token: password)
+    return build.project, build.user, :restricted if build
   end
 
   def authenticate_user(login, password)
     user = Gitlab::Auth.find_with_user_password(login, password)
     Gitlab::Auth.rate_limit!(request.ip, success: user.present?, login: login)
-    user
+    return user, :full
   end
 end
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index f5ce63fdfed..0f72dc8437c 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :user
+  attr_reader :user, :access_type
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -34,6 +34,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
         @user = auth_result.user
       end
 
+      @access_type = auth_result.access_type
+
       if ci? || user
         return # Allow access
       end
@@ -118,6 +120,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @ci.present?
   end
 
+  def full?
+    @access_type == :full
+  end
+
+  def restricted?
+    @access_type == :restricted
+  end
+
   def verify_workhorse_api!
     Gitlab::Workhorse.verify_api_request!(request.headers)
   end
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 9805705c4e3..d59a47417f4 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -86,7 +86,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access
-    @access ||= Gitlab::GitAccess.new(user, project, 'http')
+    @access ||= Gitlab::GitAccess.new(user, project, 'http', access_type: access_type)
   end
 
   def access_check
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 5d82abfca79..625dfddcf8d 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,13 +25,25 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    project.public? || ci? || (user && user.can?(:download_code, project))
+    project.public? || ci? || privileged_user_can_download_code? || restricted_user_can_download_code?
+  end
+
+  def privileged_user_can_download_code?
+    full? && user && user.can?(:download_code, project)
+  end
+
+  def restricted_user_can_download_code?
+    restricted? && user && user.can?(:restricted_download_code, project)
   end
 
   def lfs_upload_access?
     return false unless project.lfs_enabled?
 
-    user && user.can?(:push_code, project)
+    privileged_user_can_push_code?
+  end
+
+  def privileged_user_can_push_code?
+    full? && user && user.can?(:push_code, project)
   end
 
   def render_lfs_forbidden
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 61052437318..1c2e0f1edea 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -1,5 +1,7 @@
 module Ci
   class Build < CommitStatus
+    include TokenAuthenticatable
+
     belongs_to :runner, class_name: 'Ci::Runner'
     belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
     belongs_to :erased_by, class_name: 'User'
@@ -23,7 +25,10 @@ module Ci
 
     acts_as_taggable
 
+    add_authentication_token_field :token
+
     before_save :update_artifacts_size, if: :artifacts_file_changed?
+    before_save :ensure_token
     before_destroy { project }
 
     after_create :execute_hooks
@@ -172,7 +177,7 @@ module Ci
     end
 
     def repo_url
-      auth = "gitlab-ci-token:#{token}@"
+      auth = "gitlab-ci-token:#{ensure_token}@"
       project.http_url_to_repo.sub(/^https?:\/\//) do |prefix|
         prefix + auth
       end
@@ -340,12 +345,8 @@ module Ci
       )
     end
 
-    def token
-      project.runners_token
-    end
-
     def valid_token?(token)
-      project.valid_runners_token?(token)
+      self.token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token)
     end
 
     def has_tags?
diff --git a/app/models/project.rb b/app/models/project.rb
index a6de2c48071..d7cdf8775b3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1138,12 +1138,6 @@ class Project < ActiveRecord::Base
     self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
   end
 
-  # TODO (ayufan): For now we use runners_token (backward compatibility)
-  # In 8.4 every build will have its own individual token valid for time of build
-  def valid_build_token?(token)
-    self.builds_enabled? && self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
-  end
-
   def build_coverage_enabled?
     build_coverage_regex.present?
   end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index acf36d422d1..cda83bcc74a 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -64,6 +64,12 @@ class ProjectPolicy < BasePolicy
     can! :read_deployment
   end
 
+  # Permissions given when an user is direct member of a group
+  def restricted_reporter_access!
+    can! :restricted_download_code
+    can! :restricted_read_container_image
+  end
+
   def developer_access!
     can! :admin_merge_request
     can! :update_merge_request
@@ -130,10 +136,11 @@ class ProjectPolicy < BasePolicy
   def team_access!(user)
     access = project.team.max_member_access(user.id)
 
-    guest_access!     if access >= Gitlab::Access::GUEST
-    reporter_access!  if access >= Gitlab::Access::REPORTER
-    developer_access! if access >= Gitlab::Access::DEVELOPER
-    master_access!    if access >= Gitlab::Access::MASTER
+    guest_access!               if access >= Gitlab::Access::GUEST
+    reporter_access!            if access >= Gitlab::Access::REPORTER
+    restricted_reporter_access! if access >= Gitlab::Access::REPORTER
+    developer_access!           if access >= Gitlab::Access::DEVELOPER
+    master_access!              if access >= Gitlab::Access::MASTER
   end
 
   def archived_access!
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 6072123b851..270d5a11d9e 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -4,7 +4,9 @@ module Auth
 
     AUDIENCE = 'container_registry'
 
-    def execute
+    def execute(access_type: access_type)
+      @access_type = access_type
+
       return error('not found', 404) unless registry.enabled
 
       unless current_user || project
@@ -74,9 +76,9 @@ module Auth
 
       case requested_action
       when 'pull'
-        requested_project == project || can?(current_user, :read_container_image, requested_project)
+        restricted_user_can_pull?(requested_project) || privileged_user_can_pull?(requested_project)
       when 'push'
-        requested_project == project || can?(current_user, :create_container_image, requested_project)
+        restricted_user_can_push?(requested_project) || privileged_user_can_push?(requested_project)
       else
         false
       end
@@ -85,5 +87,37 @@ module Auth
     def registry
       Gitlab.config.registry
     end
+
+    private
+
+    def restricted_user_can_pull?(requested_project)
+      return false unless restricted?
+
+      # Restricted can:
+      # 1. pull from it's own project (for ex. a build)
+      # 2. read images from dependent projects if he is a team member
+      requested_project == project || can?(current_user, :restricted_read_container_image, requested_project)
+    end
+
+    def privileged_user_can_pull?(requested_project)
+      full? && can?(current_user, :read_container_image, requested_project)
+    end
+
+    def restricted_user_can_push?(requested_project)
+      # Restricted can push only to project to from which he originates
+      restricted? && requested_project == project
+    end
+
+    def privileged_user_can_push?(requested_project)
+      full? && can?(current_user, :create_container_image, requested_project)
+    end
+
+    def full?
+      @access_type == :full
+    end
+
+    def restricted?
+      @access_type == :restricted
+    end
   end
 end
diff --git a/db/migrate/20160808085531_add_token_to_build.rb b/db/migrate/20160808085531_add_token_to_build.rb
new file mode 100644
index 00000000000..3ed2a103ae3
--- /dev/null
+++ b/db/migrate/20160808085531_add_token_to_build.rb
@@ -0,0 +1,10 @@
+class AddTokenToBuild < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :ci_builds, :token, :string
+  end
+end
diff --git a/db/migrate/20160808085602_add_index_for_build_token.rb b/db/migrate/20160808085602_add_index_for_build_token.rb
new file mode 100644
index 00000000000..10ef42afce1
--- /dev/null
+++ b/db/migrate/20160808085602_add_index_for_build_token.rb
@@ -0,0 +1,12 @@
+class AddIndexForBuildToken < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def change
+    add_concurrent_index :ci_builds, :token, unique: true
+  end
+end
diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb
index bcabf7a21b2..411e0dea15e 100644
--- a/lib/ci/api/helpers.rb
+++ b/lib/ci/api/helpers.rb
@@ -14,12 +14,20 @@ module Ci
       end
 
       def authenticate_build_token!(build)
-        token = (params[BUILD_TOKEN_PARAM] || env[BUILD_TOKEN_HEADER]).to_s
-        forbidden! unless token && build.valid_token?(token)
+        forbidden! unless build_token_valid?
       end
 
       def runner_registration_token_valid?
-        params[:token] == current_application_settings.runners_registration_token
+        ActiveSupport::SecurityUtils.variable_size_secure_compare(
+          params[:token],
+          current_application_settings.runners_registration_token)
+      end
+
+      def build_token_valid?
+        token = (params[BUILD_TOKEN_PARAM] || env[BUILD_TOKEN_HEADER]).to_s
+
+        # We require to also check `runners_token` to maintain compatibility with old version of runners
+        token && (build.valid_token?(token) || build.project.valid_runners_token?(token))
       end
 
       def update_runner_last_contact(save: true)
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 91f0270818a..e7bf8ee6166 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:user, :type)
+    Result = Struct.new(:user, :type, :access_type)
 
     class << self
       def find_for_git_client(login, password, project:, ip:)
@@ -64,9 +64,7 @@ module Gitlab
 
         underscored_service = matched_login['service'].underscore
 
-        if underscored_service == 'gitlab_ci'
-          project && project.valid_build_token?(password)
-        elsif Service.available_services_names.include?(underscored_service)
+        if Service.available_services_names.include?(underscored_service)
           # We treat underscored_service as a trusted input because it is included
           # in the Service.available_services_names whitelist.
           service = project.public_send("#{underscored_service}_service")
@@ -77,12 +75,13 @@ module Gitlab
 
       def populate_result(login, password)
         result =
+          build_access_token_check(login, password) ||
           user_with_password_for_git(login, password) ||
           oauth_access_token_check(login, password) ||
           personal_access_token_check(login, password)
 
         if result
-          result.type = nil unless result.user
+          result.type = nil unless result.user && result.type != :ci
 
           if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
             result.type = :missing_personal_token
@@ -94,7 +93,7 @@ module Gitlab
 
       def user_with_password_for_git(login, password)
         user = find_with_user_password(login, password)
-        Result.new(user, :gitlab_or_ldap) if user
+        Result.new(user, :gitlab_or_ldap, :full) if user
       end
 
       def oauth_access_token_check(login, password)
@@ -102,7 +101,7 @@ module Gitlab
           token = Doorkeeper::AccessToken.by_token(password)
           if token && token.accessible?
             user = User.find_by(id: token.resource_owner_id)
-            Result.new(user, :oauth)
+            Result.new(user, :oauth, :full)
           end
         end
       end
@@ -111,7 +110,23 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, :personal_token) if user == validation
+          Result.new(user, :personal_token, :full) if user == validation
+        end
+      end
+
+      def build_access_token_check(login, password)
+        return unless login == 'gitlab-ci-token'
+        return unless password
+
+        build = Ci::Build.running.find_by_token(password)
+        return unless build
+
+        if build.user
+          # If user is assigned to build, use restricted credentials of user
+          Result.new(build.user, :build, :restricted)
+        else
+          # Otherwise use generic CI credentials (backward compatibility)
+          Result.new(nil, :ci, :restricted)
         end
       end
     end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 1882eb8d050..5bd0134ed45 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -5,12 +5,13 @@ module Gitlab
     DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
     PUSH_COMMANDS = %w{ git-receive-pack }
 
-    attr_reader :actor, :project, :protocol, :user_access
+    attr_reader :actor, :project, :protocol, :user_access, :access_type
 
-    def initialize(actor, project, protocol)
+    def initialize(actor, project, protocol, access_type: access_type)
       @actor    = actor
       @project  = project
       @protocol = protocol
+      @access_type = access_type
       @user_access = UserAccess.new(user, project: project)
     end
 
@@ -60,14 +61,26 @@ module Gitlab
     end
 
     def user_download_access_check
-      unless user_access.can_do_action?(:download_code)
+      unless privileged_user_can_download_code? || restricted_user_can_download_code?
         return build_status_object(false, "You are not allowed to download code from this project.")
       end
 
       build_status_object(true)
     end
 
+    def privileged_user_can_download_code?
+      access_type == :full && user_access.can_do_action?(:download_code)
+    end
+
+    def restricted_user_can_download_code?
+      access_type == :restricted && user_access.can_do_action?(:restricted_download_code)
+    end
+
     def user_push_access_check(changes)
+      unless access_type == :full
+        return build_status_object(false, "You are not allowed to upload code for this project.")
+      end
+
       if changes.blank?
         return build_status_object(true)
       end

From 571226f166f638f821ce84b90bce9cec1e5d5d06 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Tue, 13 Sep 2016 15:27:05 +0200
Subject: [PATCH 02/41] Make result to return project and capabilities granted

---
 app/controllers/jwt_controller.rb             | 30 ++++++----------
 .../projects/git_http_client_controller.rb    | 12 +++----
 .../projects/git_http_controller.rb           |  2 +-
 app/helpers/lfs_helper.rb                     |  6 ++--
 ...ntainer_registry_authentication_service.rb | 23 +++++-------
 lib/gitlab/auth.rb                            | 35 ++++++++++++++-----
 lib/gitlab/git_access.rb                      | 12 +++----
 7 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index ca02df28b91..1b075cc5e2d 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -11,7 +11,7 @@ class JwtController < ApplicationController
     service = SERVICES[params[:service]]
     return head :not_found unless service
 
-    result = service.new(@project, @user, auth_params).execute(access_type: @access_type)
+    result = service.new(@project, @user, auth_params).execute(capabilities: @capabilities)
 
     render json: result, status: result[:http_status]
   end
@@ -20,12 +20,16 @@ class JwtController < ApplicationController
 
   def authenticate_project_or_user
     authenticate_with_http_basic do |login, password|
-      # if it's possible we first try to authenticate project with login and password
-      @project, @user, @access_type = authenticate_build(login, password)
-      return if @project
+      @auth_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip)
 
-      @user, @access_type = authenticate_user(login, password)
-      return if @user
+      @user = auth_result.user
+      @project = auth_result.project
+      @type = auth_result.type
+      @capabilities = auth_result.capabilities || []
+
+      if @user || @project
+        return # Allow access
+      end
 
       render_403
     end
@@ -34,18 +38,4 @@ class JwtController < ApplicationController
   def auth_params
     params.permit(:service, :scope, :account, :client_id)
   end
-
-  def authenticate_build(login, password)
-    return unless login == 'gitlab-ci-token'
-    return unless password
-
-    build = Ci::Build.running.find_by(token: password)
-    return build.project, build.user, :restricted if build
-  end
-
-  def authenticate_user(login, password)
-    user = Gitlab::Auth.find_with_user_password(login, password)
-    Gitlab::Auth.rate_limit!(request.ip, success: user.present?, login: login)
-    return user, :full
-  end
 end
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 0f72dc8437c..6870102c296 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :user, :access_type
+  attr_reader :user, :capabilities
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -34,7 +34,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
         @user = auth_result.user
       end
 
-      @access_type = auth_result.access_type
+      @capabilities = auth_result.capabilities || []
 
       if ci? || user
         return # Allow access
@@ -120,12 +120,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @ci.present?
   end
 
-  def full?
-    @access_type == :full
-  end
-
-  def restricted?
-    @access_type == :restricted
+  def has_capability?(capability)
+    @capabilities.include?(capability)
   end
 
   def verify_workhorse_api!
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index d59a47417f4..89afaaed510 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -86,7 +86,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access
-    @access ||= Gitlab::GitAccess.new(user, project, 'http', access_type: access_type)
+    @access ||= Gitlab::GitAccess.new(user, project, 'http', capabilities: capabilities)
   end
 
   def access_check
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 625dfddcf8d..bee03ffb446 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -29,11 +29,11 @@ module LfsHelper
   end
 
   def privileged_user_can_download_code?
-    full? && user && user.can?(:download_code, project)
+    has_capability?(:download_code) && user && user.can?(:download_code, project)
   end
 
   def restricted_user_can_download_code?
-    restricted? && user && user.can?(:restricted_download_code, project)
+    has_capability?(:restricted_download_code) && user && user.can?(:restricted_download_code, project)
   end
 
   def lfs_upload_access?
@@ -43,7 +43,7 @@ module LfsHelper
   end
 
   def privileged_user_can_push_code?
-    full? && user && user.can?(:push_code, project)
+    has_capability?(:push_code) && user && user.can?(:push_code, project)
   end
 
   def render_lfs_forbidden
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 270d5a11d9e..cba0e2297a8 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -4,8 +4,8 @@ module Auth
 
     AUDIENCE = 'container_registry'
 
-    def execute(access_type: access_type)
-      @access_type = access_type
+    def execute(capabilities: capabilities)
+      @capabilities = capabilities
 
       return error('not found', 404) unless registry.enabled
 
@@ -91,33 +91,28 @@ module Auth
     private
 
     def restricted_user_can_pull?(requested_project)
-      return false unless restricted?
-
       # Restricted can:
       # 1. pull from it's own project (for ex. a build)
       # 2. read images from dependent projects if he is a team member
-      requested_project == project || can?(current_user, :restricted_read_container_image, requested_project)
+      requested_project == project ||
+        has_ability?(:restricted_read_container_image, requested_project)
     end
 
     def privileged_user_can_pull?(requested_project)
-      full? && can?(current_user, :read_container_image, requested_project)
+      has_ability?(:read_container_image, requested_project)
     end
 
     def restricted_user_can_push?(requested_project)
       # Restricted can push only to project to from which he originates
-      restricted? && requested_project == project
+      requested_project == project
     end
 
     def privileged_user_can_push?(requested_project)
-      full? && can?(current_user, :create_container_image, requested_project)
+      has_ability?(:create_container_image, requested_project)
     end
 
-    def full?
-      @access_type == :full
-    end
-
-    def restricted?
-      @access_type == :restricted
+    def has_ability?(ability, requested_project)
+      @capabilities.include?(ability) && can?(current_user, ability, requested_project)
     end
   end
 end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index e7bf8ee6166..001917211a1 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:user, :type, :access_type)
+    Result = Struct.new(:user, :type, :project, :capabilities)
 
     class << self
       def find_for_git_client(login, password, project:, ip:)
@@ -9,7 +9,7 @@ module Gitlab
         result = Result.new
 
         if valid_ci_request?(login, password, project)
-          result.type = :ci
+          result = Result.new(nil, project, :ci, restricted_capabilities)
         else
           result = populate_result(login, password)
         end
@@ -81,7 +81,7 @@ module Gitlab
           personal_access_token_check(login, password)
 
         if result
-          result.type = nil unless result.user && result.type != :ci
+          result.type = nil unless result.capabilities
 
           if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
             result.type = :missing_personal_token
@@ -93,7 +93,7 @@ module Gitlab
 
       def user_with_password_for_git(login, password)
         user = find_with_user_password(login, password)
-        Result.new(user, :gitlab_or_ldap, :full) if user
+        Result.new(user, :gitlab_or_ldap, nil, full_capabilities) if user
       end
 
       def oauth_access_token_check(login, password)
@@ -101,7 +101,7 @@ module Gitlab
           token = Doorkeeper::AccessToken.by_token(password)
           if token && token.accessible?
             user = User.find_by(id: token.resource_owner_id)
-            Result.new(user, :oauth, :full)
+            Result.new(user, nil, :oauth, full_capabilities)
           end
         end
       end
@@ -110,7 +110,7 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, :personal_token, :full) if user == validation
+          Result.new(user, nil, :personal_token, full_capabilities) if user == validation
         end
       end
 
@@ -123,12 +123,31 @@ module Gitlab
 
         if build.user
           # If user is assigned to build, use restricted credentials of user
-          Result.new(build.user, :build, :restricted)
+          Result.new(build.user, build.project, :build, restricted_capabilities)
         else
           # Otherwise use generic CI credentials (backward compatibility)
-          Result.new(nil, :ci, :restricted)
+          Result.new(nil, build.project, :ci, restricted_capabilities)
         end
       end
+
+      private
+
+      def restricted_capabilities
+        [
+          :read_project,
+          :restricted_download_code,
+          :restricted_read_container_image
+        ]
+      end
+
+      def full_capabilities
+        restricted_capabilities + [
+          :download_code,
+          :push_code,
+          :read_container_image,
+          :update_container_image
+        ]
+      end
     end
   end
 end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 5bd0134ed45..10ef4a1e3cf 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -5,13 +5,13 @@ module Gitlab
     DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
     PUSH_COMMANDS = %w{ git-receive-pack }
 
-    attr_reader :actor, :project, :protocol, :user_access, :access_type
+    attr_reader :actor, :project, :protocol, :user_access, :capabilities
 
-    def initialize(actor, project, protocol, access_type: access_type)
+    def initialize(actor, project, protocol, capabilities: capabilities)
       @actor    = actor
       @project  = project
       @protocol = protocol
-      @access_type = access_type
+      @capabilities = capabilities
       @user_access = UserAccess.new(user, project: project)
     end
 
@@ -69,15 +69,15 @@ module Gitlab
     end
 
     def privileged_user_can_download_code?
-      access_type == :full && user_access.can_do_action?(:download_code)
+      capabilities.include?(:download_code) && user_access.can_do_action?(:download_code)
     end
 
     def restricted_user_can_download_code?
-      access_type == :restricted && user_access.can_do_action?(:restricted_download_code)
+      capabilities.include?(:restricted_download_code) && user_access.can_do_action?(:restricted_download_code)
     end
 
     def user_push_access_check(changes)
-      unless access_type == :full
+      unless capabilities.include?(:push_code)
         return build_status_object(false, "You are not allowed to upload code for this project.")
       end
 

From ca8ed65efc8a56aafdb2011da06dd48ec55f1e07 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Tue, 13 Sep 2016 15:28:42 +0200
Subject: [PATCH 03/41] Fix result

---
 lib/gitlab/auth.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 001917211a1..0e8559022c6 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:user, :type, :project, :capabilities)
+    Result = Struct.new(:user, :project, :type, :capabilities)
 
     class << self
       def find_for_git_client(login, password, project:, ip:)

From 113372173c757abcbebe26563a4301a3027f5037 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Tue, 13 Sep 2016 16:55:57 +0200
Subject: [PATCH 04/41] Update db/schema.rb

---
 db/schema.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/db/schema.rb b/db/schema.rb
index 5c283141084..bf913e278fe 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160902122721) do
+ActiveRecord::Schema.define(version: 20160907131111) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -181,6 +181,7 @@ ActiveRecord::Schema.define(version: 20160902122721) do
     t.string   "when"
     t.text     "yaml_variables"
     t.datetime "queued_at"
+    t.string   "token"
   end
 
   add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
@@ -192,6 +193,7 @@ ActiveRecord::Schema.define(version: 20160902122721) do
   add_index "ci_builds", ["project_id"], name: "index_ci_builds_on_project_id", using: :btree
   add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree
   add_index "ci_builds", ["status"], name: "index_ci_builds_on_status", using: :btree
+  add_index "ci_builds", ["token"], name: "index_ci_builds_on_token", unique: true, using: :btree
 
   create_table "ci_commits", force: :cascade do |t|
     t.integer  "project_id"

From 79e4bb8d0b3b74ddd185677e4828d737788c3b1a Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Wed, 14 Sep 2016 17:28:24 +0200
Subject: [PATCH 05/41] Refactor Gitlab::Auth to simplify the data flow

---
 lib/gitlab/auth.rb | 74 +++++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 34 deletions(-)

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 0e8559022c6..a792db027ff 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,21 +1,29 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:user, :project, :type, :capabilities)
+    class Result
+      attr_reader :user, :project, :type, :capabilities
+
+      def initialize?(user = nil, project = nil, type = nil, capabilities = nil)
+        @user, @project, @type, @capabilities = user, project, type, capabilities
+      end
+
+      def success?
+        user.present? || [:ci, :missing_personal_token].include?(type)
+      end
+    end
 
     class << self
       def find_for_git_client(login, password, project:, ip:)
         raise "Must provide an IP for rate limiting" if ip.nil?
 
-        result = Result.new
+        result = service_access_token_check(login, password, project) ||
+          build_access_token_check(login, password) ||
+          user_with_password_for_git(login, password) ||
+          oauth_access_token_check(login, password) ||
+          personal_access_token_check(login, password) ||
+          Result.new
 
-        if valid_ci_request?(login, password, project)
-          result = Result.new(nil, project, :ci, restricted_capabilities)
-        else
-          result = populate_result(login, password)
-        end
-
-        success = result.user.present? || [:ci, :missing_personal_token].include?(result.type)
-        rate_limit!(ip, success: success, login: login)
+        rate_limit!(ip, success: result.success?, login: login)
         result
       end
 
@@ -57,10 +65,10 @@ module Gitlab
 
       private
 
-      def valid_ci_request?(login, password, project)
+      def service_access_token_check(login, password, project)
         matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
 
-        return false unless project && matched_login.present?
+        return unless project && matched_login.present?
 
         underscored_service = matched_login['service'].underscore
 
@@ -69,31 +77,24 @@ module Gitlab
           # in the Service.available_services_names whitelist.
           service = project.public_send("#{underscored_service}_service")
 
-          service && service.activated? && service.valid_token?(password)
-        end
-      end
-
-      def populate_result(login, password)
-        result =
-          build_access_token_check(login, password) ||
-          user_with_password_for_git(login, password) ||
-          oauth_access_token_check(login, password) ||
-          personal_access_token_check(login, password)
-
-        if result
-          result.type = nil unless result.capabilities
-
-          if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
-            result.type = :missing_personal_token
+          if service && service.activated? && service.valid_token?(password)
+            Result.new(nil, project, :ci, restricted_capabilities)
           end
         end
-
-        result || Result.new
       end
 
       def user_with_password_for_git(login, password)
         user = find_with_user_password(login, password)
-        Result.new(user, :gitlab_or_ldap, nil, full_capabilities) if user
+        return unless user
+
+        type =
+          if user.two_factor_enabled?
+            :missing_personal_token
+          else
+            :gitlab_or_ldap
+          end
+
+        Result.new(user, type, nil, full_capabilities)
       end
 
       def oauth_access_token_check(login, password)
@@ -101,7 +102,7 @@ module Gitlab
           token = Doorkeeper::AccessToken.by_token(password)
           if token && token.accessible?
             user = User.find_by(id: token.resource_owner_id)
-            Result.new(user, nil, :oauth, full_capabilities)
+            Result.new(user, nil, :oauth, read_capabilities)
           end
         end
       end
@@ -140,11 +141,16 @@ module Gitlab
         ]
       end
 
-      def full_capabilities
+      def read_capabilities
         restricted_capabilities + [
           :download_code,
+          :read_container_image
+        ]
+      end
+
+      def full_capabilities
+        read_capabilities + [
           :push_code,
-          :read_container_image,
           :update_container_image
         ]
       end

From 6b381f3fdf00c7eeb971f365bde2a41f0cecf944 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 10:34:53 +0200
Subject: [PATCH 06/41] Use `build_read_container_image` and use
 `build_download_code`

---
 app/controllers/jwt_controller.rb             | 18 ++++------
 app/helpers/lfs_helper.rb                     |  8 ++---
 app/policies/project_policy.rb                | 18 +++++-----
 ...ntainer_registry_authentication_service.rb | 35 +++++++++----------
 lib/gitlab/auth.rb                            | 16 +++++----
 lib/gitlab/git_access.rb                      |  8 ++---
 6 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 1b075cc5e2d..7bf534d8732 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -11,7 +11,10 @@ class JwtController < ApplicationController
     service = SERVICES[params[:service]]
     return head :not_found unless service
 
-    result = service.new(@project, @user, auth_params).execute(capabilities: @capabilities)
+    @@authentication_result ||= Gitlab::Auth.Result.new
+
+    result = service.new(@authentication_result.project, @authentication_result.user, auth_params).
+      execute(capabilities: @authentication_result.capabilities || [])
 
     render json: result, status: result[:http_status]
   end
@@ -20,18 +23,9 @@ class JwtController < ApplicationController
 
   def authenticate_project_or_user
     authenticate_with_http_basic do |login, password|
-      @auth_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip)
+      @authentication_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip)
 
-      @user = auth_result.user
-      @project = auth_result.project
-      @type = auth_result.type
-      @capabilities = auth_result.capabilities || []
-
-      if @user || @project
-        return # Allow access
-      end
-
-      render_403
+      render_403 unless @authentication_result.success?
     end
   end
 
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index bee03ffb446..a2359d94443 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,15 +25,15 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    project.public? || ci? || privileged_user_can_download_code? || restricted_user_can_download_code?
+    project.public? || ci? || user_can_download_code? || build_can_download_code?
   end
 
-  def privileged_user_can_download_code?
+  def user_can_download_code?
     has_capability?(:download_code) && user && user.can?(:download_code, project)
   end
 
-  def restricted_user_can_download_code?
-    has_capability?(:restricted_download_code) && user && user.can?(:restricted_download_code, project)
+  def build_can_download_code?
+    has_capability?(:build_download_code) && user && user.can?(:build_download_code, project)
   end
 
   def lfs_upload_access?
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index cda83bcc74a..ce686af2ade 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -65,9 +65,9 @@ class ProjectPolicy < BasePolicy
   end
 
   # Permissions given when an user is direct member of a group
-  def restricted_reporter_access!
-    can! :restricted_download_code
-    can! :restricted_read_container_image
+  def team_member_reporter_access!
+    can! :build_download_code
+    can! :build_read_container_image
   end
 
   def developer_access!
@@ -115,6 +115,8 @@ class ProjectPolicy < BasePolicy
     can! :read_commit_status
     can! :read_pipeline
     can! :read_container_image
+    can! :build_download_code
+    can! :build_read_container_image
   end
 
   def owner_access!
@@ -136,11 +138,11 @@ class ProjectPolicy < BasePolicy
   def team_access!(user)
     access = project.team.max_member_access(user.id)
 
-    guest_access!               if access >= Gitlab::Access::GUEST
-    reporter_access!            if access >= Gitlab::Access::REPORTER
-    restricted_reporter_access! if access >= Gitlab::Access::REPORTER
-    developer_access!           if access >= Gitlab::Access::DEVELOPER
-    master_access!              if access >= Gitlab::Access::MASTER
+    guest_access!                if access >= Gitlab::Access::GUEST
+    reporter_access!             if access >= Gitlab::Access::REPORTER
+    team_member_reporter_access! if access >= Gitlab::Access::REPORTER
+    developer_access!            if access >= Gitlab::Access::DEVELOPER
+    master_access!               if access >= Gitlab::Access::MASTER
   end
 
   def archived_access!
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index cba0e2297a8..ba0b60abfe4 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -76,9 +76,9 @@ module Auth
 
       case requested_action
       when 'pull'
-        restricted_user_can_pull?(requested_project) || privileged_user_can_pull?(requested_project)
+        build_can_pull?(requested_project) || user_can_pull?(requested_project)
       when 'push'
-        restricted_user_can_push?(requested_project) || privileged_user_can_push?(requested_project)
+        build_can_push?(requested_project) || user_can_push?(requested_project)
       else
         false
       end
@@ -90,29 +90,28 @@ module Auth
 
     private
 
-    def restricted_user_can_pull?(requested_project)
-      # Restricted can:
+    def build_can_pull?(requested_project)
+      # Build can:
       # 1. pull from it's own project (for ex. a build)
-      # 2. read images from dependent projects if he is a team member
-      requested_project == project ||
-        has_ability?(:restricted_read_container_image, requested_project)
+      # 2. read images from dependent projects if creator of build is a team member
+      @capabilities.include?(:build_read_container_image) &&
+        (requested_project == project || can?(current_user, :build_read_container_image, requested_project))
     end
 
-    def privileged_user_can_pull?(requested_project)
-      has_ability?(:read_container_image, requested_project)
+    def user_can_pull?(requested_project)
+      @capabilities.include?(:read_container_image) &&
+        can?(current_user, :read_container_image, requested_project)
     end
 
-    def restricted_user_can_push?(requested_project)
-      # Restricted can push only to project to from which he originates
-      requested_project == project
+    def build_can_push?(requested_project)
+      # Build can push only to project to from which he originates
+      @capabilities.include?(:build_create_container_image) &&
+        requested_project == project
     end
 
-    def privileged_user_can_push?(requested_project)
-      has_ability?(:create_container_image, requested_project)
-    end
-
-    def has_ability?(ability, requested_project)
-      @capabilities.include?(ability) && can?(current_user, ability, requested_project)
+    def user_can_push?(requested_project)
+      @capabilities.include?(:create_container_image) &&
+        can?(current_user, :create_container_image, requested_project)
     end
   end
 end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index a792db027ff..6a55c50c3f3 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -78,7 +78,7 @@ module Gitlab
           service = project.public_send("#{underscored_service}_service")
 
           if service && service.activated? && service.valid_token?(password)
-            Result.new(nil, project, :ci, restricted_capabilities)
+            Result.new(nil, project, :ci, build_capabilities)
           end
         end
       end
@@ -124,25 +124,27 @@ module Gitlab
 
         if build.user
           # If user is assigned to build, use restricted credentials of user
-          Result.new(build.user, build.project, :build, restricted_capabilities)
+          Result.new(build.user, build.project, :build, build_capabilities)
         else
           # Otherwise use generic CI credentials (backward compatibility)
-          Result.new(nil, build.project, :ci, restricted_capabilities)
+          Result.new(nil, build.project, :ci, build_capabilities)
         end
       end
 
       private
 
-      def restricted_capabilities
+      def build_capabilities
         [
           :read_project,
-          :restricted_download_code,
-          :restricted_read_container_image
+          :build_download_code,
+          :build_read_container_image,
+          :build_create_container_image
         ]
       end
 
       def read_capabilities
-        restricted_capabilities + [
+        [
+          :read_project,
           :download_code,
           :read_container_image
         ]
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 10ef4a1e3cf..63b707db814 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -61,19 +61,19 @@ module Gitlab
     end
 
     def user_download_access_check
-      unless privileged_user_can_download_code? || restricted_user_can_download_code?
+      unless user_can_download_code? || build_can_download_code?
         return build_status_object(false, "You are not allowed to download code from this project.")
       end
 
       build_status_object(true)
     end
 
-    def privileged_user_can_download_code?
+    def user_can_download_code?
       capabilities.include?(:download_code) && user_access.can_do_action?(:download_code)
     end
 
-    def restricted_user_can_download_code?
-      capabilities.include?(:restricted_download_code) && user_access.can_do_action?(:restricted_download_code)
+    def build_can_download_code?
+      capabilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
     def user_push_access_check(changes)

From 11f87700e8bceeec96440809682406ae24334ed8 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 11:57:09 +0200
Subject: [PATCH 07/41] Add access specs

---
 app/controllers/jwt_controller.rb  |   2 +-
 lib/api/internal.rb                |  14 +++-
 lib/gitlab/auth.rb                 |  20 +++---
 spec/lib/gitlab/auth_spec.rb       |  76 ++++++++++++++++++--
 spec/lib/gitlab/git_access_spec.rb | 111 +++++++++++++++++++++--------
 5 files changed, 173 insertions(+), 50 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 7bf534d8732..ed5d28e0d2c 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -25,7 +25,7 @@ class JwtController < ApplicationController
     authenticate_with_http_basic do |login, password|
       @authentication_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip)
 
-      render_403 unless @authentication_result.success?
+      render_403 unless @authentication_result.succeeded?
     end
   end
 
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 6e6efece7c4..2ec94570506 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -51,9 +51,9 @@ module API
 
         access =
           if wiki?
-            Gitlab::GitAccessWiki.new(actor, project, protocol)
+            Gitlab::GitAccessWiki.new(actor, project, protocol, capabilities: ssh_capabilities)
           else
-            Gitlab::GitAccess.new(actor, project, protocol)
+            Gitlab::GitAccess.new(actor, project, protocol, capabilities: ssh_capabilities)
           end
 
         access_status = access.check(params[:action], params[:changes])
@@ -130,6 +130,16 @@ module API
 
         { success: true, recovery_codes: codes }
       end
+
+      private
+
+      def ssh_capabilities
+        [
+          :read_project,
+          :download_code,
+          :push_code
+        ]
+      end
     end
   end
 end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 6a55c50c3f3..7af9bb9a326 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,14 +1,8 @@
 module Gitlab
   module Auth
-    class Result
-      attr_reader :user, :project, :type, :capabilities
-
-      def initialize?(user = nil, project = nil, type = nil, capabilities = nil)
-        @user, @project, @type, @capabilities = user, project, type, capabilities
-      end
-
-      def success?
-        user.present? || [:ci, :missing_personal_token].include?(type)
+    Result = Struct.new(:user, :project, :type, :capabilities) do
+      def succeeded?
+        user.present? || [:ci].include?(type)
       end
     end
 
@@ -23,7 +17,7 @@ module Gitlab
           personal_access_token_check(login, password) ||
           Result.new
 
-        rate_limit!(ip, success: result.success?, login: login)
+        rate_limit!(ip, success: result.succeeded?, login: login)
         result
       end
 
@@ -94,7 +88,7 @@ module Gitlab
             :gitlab_or_ldap
           end
 
-        Result.new(user, type, nil, full_capabilities)
+        Result.new(user, nil, type, full_capabilities)
       end
 
       def oauth_access_token_check(login, password)
@@ -111,7 +105,9 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, nil, :personal_token, full_capabilities) if user == validation
+          if user && user == validation
+            Result.new(user, nil, :personal_token, full_capabilities)
+          end
         end
       end
 
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 7c23e02d05a..b665517bbb0 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -4,15 +4,51 @@ describe Gitlab::Auth, lib: true do
   let(:gl_auth) { described_class }
 
   describe 'find_for_git_client' do
-    it 'recognizes CI' do
-      token = '123'
+    context 'build token' do
+      subject { gl_auth.find_for_git_client('gitlab-ci-token', build.token, project: project, ip: 'ip') }
+
+      context 'for running build' do
+        let!(:build) { create(:ci_build, :running) }
+        let(:project) { build.project }
+
+        before do
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'gitlab-ci-token')
+        end
+
+        it 'recognises user-less build' do
+          expect(subject).to eq(Gitlab::Auth::Result.new(nil, build.project, :ci, build_capabilities))
+        end
+
+        it 'recognises user token' do
+          build.update(user: create(:user))
+
+          expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, build_capabilities))
+        end
+      end
+
+      context 'for non-running build' do
+        let!(:build) { create(:ci_build, :pending) }
+        let(:project) { build.project }
+
+        before do
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'gitlab-ci-token')
+        end
+
+        it 'denies authentication' do
+          expect(subject).to eq(Gitlab::Auth::Result.new)
+        end
+      end
+    end
+
+    it 'recognizes other ci services' do
       project = create(:empty_project)
-      project.update_attributes(runners_token: token)
+      project.create_drone_ci_service(active: true)
+      project.drone_ci_service.update(token: 'token')
 
       ip = 'ip'
 
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'gitlab-ci-token')
-      expect(gl_auth.find_for_git_client('gitlab-ci-token', token, project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, :ci))
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'drone-ci-token')
+      expect(gl_auth.find_for_git_client('drone-ci-token', 'token', project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, project, :ci, build_capabilities))
     end
 
     it 'recognizes master passwords' do
@@ -20,7 +56,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
+      expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_capabilities))
     end
 
     it 'recognizes OAuth tokens' do
@@ -30,7 +66,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2')
-      expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :oauth))
+      expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_capabilities))
     end
 
     it 'returns double nil for invalid credentials' do
@@ -92,4 +128,30 @@ describe Gitlab::Auth, lib: true do
       end
     end
   end
+
+  private
+
+  def build_capabilities
+    [
+      :read_project,
+      :build_download_code,
+      :build_read_container_image,
+      :build_create_container_image
+    ]
+  end
+
+  def read_capabilities
+    [
+      :read_project,
+      :download_code,
+      :read_container_image
+    ]
+  end
+
+  def full_capabilities
+    read_capabilities + [
+      :push_code,
+      :update_container_image
+    ]
+  end
 end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index f12c9a370f7..77dce676cdb 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -1,10 +1,17 @@
 require 'spec_helper'
 
 describe Gitlab::GitAccess, lib: true do
-  let(:access) { Gitlab::GitAccess.new(actor, project, 'web') }
+  let(:access) { Gitlab::GitAccess.new(actor, project, 'web', capabilities: capabilities) }
   let(:project) { create(:project) }
   let(:user) { create(:user) }
   let(:actor) { user }
+  let(:capabilities) do
+    [
+      :read_project,
+      :download_code,
+      :push_code
+    ]
+  end
 
   describe '#check with single protocols allowed' do
     def disable_protocol(protocol)
@@ -111,6 +118,36 @@ describe Gitlab::GitAccess, lib: true do
         end
       end
     end
+
+    describe 'build capabilities permissions' do
+      let(:capabilities) { build_capabilities }
+
+      describe 'reporter user' do
+        before { project.team << [user, :reporter] }
+
+        context 'pull code' do
+          it { expect(subject).to be_allowed }
+        end
+      end
+
+      describe 'admin user' do
+        let(:user) { create(:admin) }
+
+        context 'when member of the project' do
+          before { project.team << [user, :reporter] }
+
+          context 'pull code' do
+            it { expect(subject).to be_allowed }
+          end
+        end
+
+        context 'when is not member of the project' do
+          context 'pull code' do
+            it { expect(subject).not_to be_allowed }
+          end
+        end
+      end
+    end
   end
 
   describe 'push_access_check' do
@@ -281,40 +318,58 @@ describe Gitlab::GitAccess, lib: true do
                                                             admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }))
       end
     end
+
+  end
+
+  shared_examples 'can not push code' do
+    subject { access.check('git-receive-pack', '_any') }
+
+    context 'when project is authorized' do
+      before { key.projects << project }
+
+      it { expect(subject).not_to be_allowed }
+    end
+
+    context 'when unauthorized' do
+      context 'to public project' do
+        let(:project) { create(:project, :public) }
+
+        it { expect(subject).not_to be_allowed }
+      end
+
+      context 'to internal project' do
+        let(:project) { create(:project, :internal) }
+
+        it { expect(subject).not_to be_allowed }
+      end
+
+      context 'to private project' do
+        let(:project) { create(:project, :internal) }
+
+        it { expect(subject).not_to be_allowed }
+      end
+    end
+  end
+
+  describe 'build capabilities permissions' do
+    let(:capabilities) { build_capabilities }
+
+    it_behaves_like 'cannot push code'
   end
 
   describe 'deploy key permissions' do
     let(:key) { create(:deploy_key) }
     let(:actor) { key }
 
-    context 'push code' do
-      subject { access.check('git-receive-pack', '_any') }
+    it_behaves_like 'cannot push code'
+  end
 
-      context 'when project is authorized' do
-        before { key.projects << project }
+  private
 
-        it { expect(subject).not_to be_allowed }
-      end
-
-      context 'when unauthorized' do
-        context 'to public project' do
-          let(:project) { create(:project, :public) }
-
-          it { expect(subject).not_to be_allowed }
-        end
-
-        context 'to internal project' do
-          let(:project) { create(:project, :internal) }
-
-          it { expect(subject).not_to be_allowed }
-        end
-
-        context 'to private project' do
-          let(:project) { create(:project, :internal) }
-
-          it { expect(subject).not_to be_allowed }
-        end
-      end
-    end
+  def build_capabilities
+    [
+      :read_project,
+      :build_download_code
+    ]
   end
 end

From 9d1ccd2ad3af37139649100476b568d219343a57 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 13:49:11 +0200
Subject: [PATCH 08/41] Fix existing authorization specs

---
 app/controllers/jwt_controller.rb              |  6 +++---
 .../projects/git_http_client_controller.rb     |  2 +-
 app/models/ci/build.rb                         |  1 +
 ...ontainer_registry_authentication_service.rb |  8 +++-----
 lib/api/internal.rb                            | 18 ++++++++----------
 lib/gitlab/auth.rb                             |  2 +-
 lib/gitlab/git_access.rb                       |  2 +-
 spec/lib/gitlab/git_access_spec.rb             | 17 ++++++++++++-----
 spec/requests/git_http_spec.rb                 |  9 ++++-----
 spec/requests/jwt_controller_spec.rb           |  6 ++++--
 ...ner_registry_authentication_service_spec.rb | 14 +++++++++++++-
 11 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index ed5d28e0d2c..0870a2a8f50 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -11,10 +11,10 @@ class JwtController < ApplicationController
     service = SERVICES[params[:service]]
     return head :not_found unless service
 
-    @@authentication_result ||= Gitlab::Auth.Result.new
+    @authentication_result ||= Gitlab::Auth::Result.new
 
     result = service.new(@authentication_result.project, @authentication_result.user, auth_params).
-      execute(capabilities: @authentication_result.capabilities || [])
+      execute(capabilities: @authentication_result.capabilities)
 
     render json: result, status: result[:http_status]
   end
@@ -23,7 +23,7 @@ class JwtController < ApplicationController
 
   def authenticate_project_or_user
     authenticate_with_http_basic do |login, password|
-      @authentication_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip)
+      @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip)
 
       render_403 unless @authentication_result.succeeded?
     end
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 6870102c296..aabb5b0fe01 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -36,7 +36,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
       @capabilities = auth_result.capabilities || []
 
-      if ci? || user
+      if auth_result.succeeded?
         return # Allow access
       end
     elsif allow_kerberos_spnego_auth? && spnego_provided?
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 29788b8285d..0b017c98916 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -43,6 +43,7 @@ module Ci
         new_build.status = 'pending'
         new_build.runner_id = nil
         new_build.trigger_request_id = nil
+        new_build.token = nil
         new_build.save
       end
 
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index ba0b60abfe4..df1c9b2851c 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -4,8 +4,8 @@ module Auth
 
     AUDIENCE = 'container_registry'
 
-    def execute(capabilities: capabilities)
-      @capabilities = capabilities
+    def execute(capabilities:)
+      @capabilities = capabilities || []
 
       return error('not found', 404) unless registry.enabled
 
@@ -76,7 +76,7 @@ module Auth
 
       case requested_action
       when 'pull'
-        build_can_pull?(requested_project) || user_can_pull?(requested_project)
+        requested_project.public? || build_can_pull?(requested_project) || user_can_pull?(requested_project)
       when 'push'
         build_can_push?(requested_project) || user_can_push?(requested_project)
       else
@@ -88,8 +88,6 @@ module Auth
       Gitlab.config.registry
     end
 
-    private
-
     def build_can_pull?(requested_project)
       # Build can:
       # 1. pull from it's own project (for ex. a build)
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 2ec94570506..2610fd329d6 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -35,6 +35,14 @@ module API
             Project.find_with_namespace(project_path)
           end
         end
+
+        def ssh_capabilities
+          [
+            :read_project,
+            :download_code,
+            :push_code
+          ]
+        end
       end
 
       post "/allowed" do
@@ -130,16 +138,6 @@ module API
 
         { success: true, recovery_codes: codes }
       end
-
-      private
-
-      def ssh_capabilities
-        [
-          :read_project,
-          :download_code,
-          :push_code
-        ]
-      end
     end
   end
 end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 7af9bb9a326..666cf1b3e26 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -115,7 +115,7 @@ module Gitlab
         return unless login == 'gitlab-ci-token'
         return unless password
 
-        build = Ci::Build.running.find_by_token(password)
+        build = ::Ci::Build.running.find_by_token(password)
         return unless build
 
         if build.user
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 63b707db814..21286e77dc6 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -7,7 +7,7 @@ module Gitlab
 
     attr_reader :actor, :project, :protocol, :user_access, :capabilities
 
-    def initialize(actor, project, protocol, capabilities: capabilities)
+    def initialize(actor, project, protocol, capabilities:)
       @actor    = actor
       @project  = project
       @protocol = protocol
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 77dce676cdb..d418b0be0ed 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -22,7 +22,7 @@ describe Gitlab::GitAccess, lib: true do
     context 'ssh disabled' do
       before do
         disable_protocol('ssh')
-        @acc = Gitlab::GitAccess.new(actor, project, 'ssh')
+        @acc = Gitlab::GitAccess.new(actor, project, 'ssh', capabilities: capabilities)
       end
 
       it 'blocks ssh git push' do
@@ -37,7 +37,7 @@ describe Gitlab::GitAccess, lib: true do
     context 'http disabled' do
       before do
         disable_protocol('http')
-        @acc = Gitlab::GitAccess.new(actor, project, 'http')
+        @acc = Gitlab::GitAccess.new(actor, project, 'http', capabilities: capabilities)
       end
 
       it 'blocks http push' do
@@ -318,7 +318,6 @@ describe Gitlab::GitAccess, lib: true do
                                                             admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }))
       end
     end
-
   end
 
   shared_examples 'can not push code' do
@@ -354,14 +353,14 @@ describe Gitlab::GitAccess, lib: true do
   describe 'build capabilities permissions' do
     let(:capabilities) { build_capabilities }
 
-    it_behaves_like 'cannot push code'
+    it_behaves_like 'can not push code'
   end
 
   describe 'deploy key permissions' do
     let(:key) { create(:deploy_key) }
     let(:actor) { key }
 
-    it_behaves_like 'cannot push code'
+    it_behaves_like 'can not push code'
   end
 
   private
@@ -372,4 +371,12 @@ describe Gitlab::GitAccess, lib: true do
       :build_download_code
     ]
   end
+
+  def full_capabilities
+    [
+      :read_project,
+      :download_code,
+      :push_code
+    ]
+  end
 end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index b7001fede40..5977ee04524 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -300,23 +300,22 @@ describe 'Git HTTP requests', lib: true do
       end
 
       context "when a gitlab ci token is provided" do
-        let(:token) { 123 }
-        let(:project) { FactoryGirl.create :empty_project }
+        let(:build) { create(:ci_build, :running) }
+        let(:project) { build.project }
 
         before do
-          project.update_attributes(runners_token: token)
           project.project_feature.update_attributes(builds_access_level: ProjectFeature::ENABLED)
         end
 
         it "downloads get status 200" do
-          clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token
+          clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
           expect(response).to have_http_status(200)
           expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
         end
 
         it "uploads get status 401 (no project existence information leak)" do
-          push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token
+          push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
           expect(response).to have_http_status(401)
         end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index fc42b534dca..93b9cfaf33d 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -22,11 +22,13 @@ describe JwtController do
 
   context 'when using authorized request' do
     context 'using CI token' do
-      let(:project) { create(:empty_project, runners_token: 'token') }
-      let(:headers) { { authorization: credentials('gitlab-ci-token', project.runners_token) } }
+      let(:build) { create(:ci_build, :running) }
+      let(:project) { build.project }
+      let(:headers) { { authorization: credentials('gitlab-ci-token', build.token) } }
 
       context 'project with enabled CI' do
         subject! { get '/jwt/auth', parameters, headers }
+
         it { expect(service_class).to have_received(:new).with(project, nil, parameters) }
       end
 
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 7cc71f706ce..c82deb7d423 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -6,8 +6,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
   let(:current_params) { {} }
   let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) }
   let(:payload) { JWT.decode(subject[:token], rsa_key).first }
+  let(:capabilities) do
+    [
+      :read_container_image,
+      :create_container_image
+    ]
+  end
 
-  subject { described_class.new(current_project, current_user, current_params).execute }
+  subject { described_class.new(current_project, current_user, current_params).execute(capabilities: capabilities) }
 
   before do
     allow(Gitlab.config.registry).to receive_messages(enabled: true, issuer: 'rspec', key: nil)
@@ -42,6 +48,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
          'actions' => actions,
        }]
     end
+    let(:capabilities) do
+      [
+        :build_read_container_image,
+        :build_create_container_image
+      ]
+    end
 
     it_behaves_like 'a valid token'
     it { expect(payload).to include('access' => access) }

From 551787ac5c12a502b46c819939b2fa11684a799c Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 14:06:10 +0200
Subject: [PATCH 09/41] Simplify LFS helper

---
 app/helpers/lfs_helper.rb | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index a2359d94443..d27b87ed5e4 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -39,10 +39,6 @@ module LfsHelper
   def lfs_upload_access?
     return false unless project.lfs_enabled?
 
-    privileged_user_can_push_code?
-  end
-
-  def privileged_user_can_push_code?
     has_capability?(:push_code) && user && user.can?(:push_code, project)
   end
 

From 548169cfb57b27cca911d947e2aa6f4f7e6df004 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 15:40:53 +0200
Subject: [PATCH 10/41] Fix most of specs

---
 .../projects/git_http_client_controller.rb         |  7 +++++--
 lib/ci/api/helpers.rb                              |  4 ++--
 lib/gitlab/auth.rb                                 |  3 +--
 spec/lib/gitlab/git_access_spec.rb                 | 14 +++++++++++---
 spec/lib/gitlab/git_access_wiki_spec.rb            |  9 ++++++++-
 spec/requests/lfs_http_spec.rb                     | 10 ++++++----
 ...ntainer_registry_authentication_service_spec.rb | 12 ++++++------
 7 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index aabb5b0fe01..c2a298fe37f 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -23,10 +23,12 @@ class Projects::GitHttpClientController < Projects::ApplicationController
       login, password = user_name_and_password(request)
       auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
-      if auth_result.type == :ci && download_request?
-        @ci = true
+      if auth_result.type == :ci && !download_request?
+        # Not allowed
+        auth_result = Gitlab::Auth::Result.new
       elsif auth_result.type == :oauth && !download_request?
         # Not allowed
+        auth_result = Gitlab::Auth::Result.new
       elsif auth_result.type == :missing_personal_token
         render_missing_personal_token
         return # Render above denied access, nothing left to do
@@ -35,6 +37,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
       end
 
       @capabilities = auth_result.capabilities || []
+      @ci = auth_result.type == :ci
 
       if auth_result.succeeded?
         return # Allow access
diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb
index 3dfebc0cac1..23353c62885 100644
--- a/lib/ci/api/helpers.rb
+++ b/lib/ci/api/helpers.rb
@@ -14,7 +14,7 @@ module Ci
       end
 
       def authenticate_build_token!(build)
-        forbidden! unless build_token_valid?
+        forbidden! unless build_token_valid?(build)
       end
 
       def runner_registration_token_valid?
@@ -23,7 +23,7 @@ module Ci
           current_application_settings.runners_registration_token)
       end
 
-      def build_token_valid?
+      def build_token_valid?(build)
         token = (params[BUILD_TOKEN_PARAM] || env[BUILD_TOKEN_HEADER]).to_s
 
         # We require to also check `runners_token` to maintain compatibility with old version of runners
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 666cf1b3e26..b1427f412b0 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -117,6 +117,7 @@ module Gitlab
 
         build = ::Ci::Build.running.find_by_token(password)
         return unless build
+        return unless build.project.builds_enabled?
 
         if build.user
           # If user is assigned to build, use restricted credentials of user
@@ -127,8 +128,6 @@ module Gitlab
         end
       end
 
-      private
-
       def build_capabilities
         [
           :read_project,
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index d418b0be0ed..c6fe56aac1c 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -324,7 +324,7 @@ describe Gitlab::GitAccess, lib: true do
     subject { access.check('git-receive-pack', '_any') }
 
     context 'when project is authorized' do
-      before { key.projects << project }
+      before { authorize }
 
       it { expect(subject).not_to be_allowed }
     end
@@ -353,14 +353,22 @@ describe Gitlab::GitAccess, lib: true do
   describe 'build capabilities permissions' do
     let(:capabilities) { build_capabilities }
 
-    it_behaves_like 'can not push code'
+    it_behaves_like 'can not push code' do
+      def authorize
+        project.team << [user, :reporter]
+      end
+    end
   end
 
   describe 'deploy key permissions' do
     let(:key) { create(:deploy_key) }
     let(:actor) { key }
 
-    it_behaves_like 'can not push code'
+    it_behaves_like 'can not push code' do
+      def authorize
+        key.projects << project
+      end
+    end
   end
 
   private
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index 4244b807d41..860e701c1a1 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -1,9 +1,16 @@
 require 'spec_helper'
 
 describe Gitlab::GitAccessWiki, lib: true do
-  let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web') }
+  let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', capabilities: capabilities) }
   let(:project) { create(:project) }
   let(:user) { create(:user) }
+  let(:capabilities) do
+    [
+      :read_project,
+      :download_code,
+      :push_code
+    ]
+  end
 
   describe 'push_allowed?' do
     before do
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 6e551bb65fa..85290ec05c2 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -586,8 +586,8 @@ describe 'Git LFS API and storage' do
         context 'when CI is authorized' do
           let(:authorization) { authorize_ci_project }
 
-          it 'responds with 401' do
-            expect(response).to have_http_status(401)
+          it 'responds with 403' do
+            expect(response).to have_http_status(403)
           end
         end
       end
@@ -614,7 +614,7 @@ describe 'Git LFS API and storage' do
         let(:authorization) { authorize_ci_project }
 
         it 'responds with status 403' do
-          expect(response).to have_http_status(401)
+          expect(response).to have_http_status(403)
         end
       end
     end
@@ -897,7 +897,9 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_ci_project
-    ActionController::HttpAuthentication::Basic.encode_credentials('gitlab-ci-token', project.runners_token)
+    pipeline = create(:ci_empty_pipeline, project: project)
+    build = create(:ci_build, :running, pipeline: pipeline)
+    ActionController::HttpAuthentication::Basic.encode_credentials('gitlab-ci-token', build.token)
   end
 
   def authorize_user
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index c82deb7d423..5f82fee43c6 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -48,12 +48,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
          'actions' => actions,
        }]
     end
-    let(:capabilities) do
-      [
-        :build_read_container_image,
-        :build_create_container_image
-      ]
-    end
 
     it_behaves_like 'a valid token'
     it { expect(payload).to include('access' => access) }
@@ -203,6 +197,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
 
   context 'project authorization' do
     let(:current_project) { create(:empty_project) }
+    let(:capabilities) do
+      [
+        :build_read_container_image,
+        :build_create_container_image
+      ]
+    end
 
     context 'allow to use scope-less authentication' do
       it_behaves_like 'a valid token'

From e3a422c2672096a819291c395623619c8c669e74 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 16:17:57 +0200
Subject: [PATCH 11/41] Fix LFS specs

---
 spec/requests/lfs_http_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 85290ec05c2..1ee3881b839 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -586,8 +586,8 @@ describe 'Git LFS API and storage' do
         context 'when CI is authorized' do
           let(:authorization) { authorize_ci_project }
 
-          it 'responds with 403' do
-            expect(response).to have_http_status(403)
+          it 'responds with 401' do
+            expect(response).to have_http_status(401)
           end
         end
       end
@@ -613,8 +613,8 @@ describe 'Git LFS API and storage' do
       context 'when CI is authorized' do
         let(:authorization) { authorize_ci_project }
 
-        it 'responds with status 403' do
-          expect(response).to have_http_status(403)
+        it 'responds with status 401' do
+          expect(response).to have_http_status(401)
         end
       end
     end

From eed5c58d8542cef8cc4012a303c9bb963b7f5f20 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 16:36:39 +0200
Subject: [PATCH 12/41] Verify permission of build in context of dependent
 project

---
 spec/requests/lfs_http_spec.rb                |  6 +--
 ...er_registry_authentication_service_spec.rb | 40 ++++++++++++++++---
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 1ee3881b839..7bf43a03f23 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -14,6 +14,8 @@ describe 'Git LFS API and storage' do
   end
   let(:authorization) { }
   let(:sendfile) { }
+  let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+  let(:build) { create(:ci_build, :running, pipeline: pipeline) }
 
   let(:sample_oid) { lfs_object.oid }
   let(:sample_size) { lfs_object.size }
@@ -244,7 +246,7 @@ describe 'Git LFS API and storage' do
           end
         end
 
-        context 'when CI is authorized' do
+        context 'when build is authorized' do
           let(:authorization) { authorize_ci_project }
 
           let(:update_permissions) do
@@ -897,8 +899,6 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_ci_project
-    pipeline = create(:ci_empty_pipeline, project: project)
-    build = create(:ci_build, :running, pipeline: pipeline)
     ActionController::HttpAuthentication::Basic.encode_credentials('gitlab-ci-token', build.token)
   end
 
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 5f82fee43c6..2d39bd61b8f 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -195,8 +195,9 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
     end
   end
 
-  context 'project authorization' do
+  context 'build authorized as user' do
     let(:current_project) { create(:empty_project) }
+    let(:current_user) { create(:user) }
     let(:capabilities) do
       [
         :build_read_container_image,
@@ -204,10 +205,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
       ]
     end
 
-    context 'allow to use scope-less authentication' do
-      it_behaves_like 'a valid token'
+    before do
+      current_project.team << [current_user, :developer]
     end
 
+    it_behaves_like 'a valid token'
+
     context 'allow to pull and push images' do
       let(:current_params) do
         { scope: "repository:#{current_project.path_with_namespace}:pull,push" }
@@ -226,12 +229,34 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
 
         context 'allow for public' do
           let(:project) { create(:empty_project, :public) }
+
           it_behaves_like 'a pullable'
         end
 
-        context 'disallow for private' do
+        shared_examples 'pullable for being team member' do
+          context 'when you are not member' do
+            it_behaves_like 'an inaccessible'
+          end
+
+          context 'when you are member' do
+            before do
+              project.team << [current_user, :developer]
+            end
+
+            it_behaves_like 'a pullable'
+          end
+        end
+
+        context 'for private' do
           let(:project) { create(:empty_project, :private) }
-          it_behaves_like 'an inaccessible'
+
+          it_behaves_like 'pullable for being team member'
+
+          context 'when you are admin' do
+            let(:current_user) { create(:admin) }
+
+            it_behaves_like 'pullable for being team member'
+          end
         end
       end
 
@@ -242,6 +267,11 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
 
         context 'disallow for all' do
           let(:project) { create(:empty_project, :public) }
+
+          before do
+            project.team << [current_user, :developer]
+          end
+
           it_behaves_like 'an inaccessible'
         end
       end

From e40e3fdc8271d1becf7952c7e30546c5abecb79b Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 25 Aug 2016 17:26:20 -0500
Subject: [PATCH 13/41] Added LFS support to SSH - Required on the GitLab Rails
 side is mostly authentication and API related.

---
 .../projects/git_http_client_controller.rb    | 40 ++++++++++++++-----
 app/helpers/lfs_helper.rb                     |  2 +-
 app/models/deploy_key.rb                      |  5 +++
 app/models/user.rb                            |  3 +-
 .../20160825173042_add_lfs_token_to_users.rb  | 16 ++++++++
 .../20160825182924_add_lfs_token_to_keys.rb   | 16 ++++++++
 lib/api/entities.rb                           |  2 +-
 lib/api/internal.rb                           | 13 +++++-
 lib/gitlab/auth.rb                            | 13 +++++-
 spec/lib/gitlab/auth_spec.rb                  | 16 ++++++++
 .../concerns/token_authenticatable_spec.rb    | 20 ++++++++++
 spec/requests/api/internal_spec.rb            | 26 ++++++++++--
 spec/requests/lfs_http_spec.rb                | 16 ++++++++
 13 files changed, 168 insertions(+), 20 deletions(-)
 create mode 100644 db/migrate/20160825173042_add_lfs_token_to_users.rb
 create mode 100644 db/migrate/20160825182924_add_lfs_token_to_keys.rb

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index f5ce63fdfed..1e6709dc8eb 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,6 +4,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
+  class MissingPersonalTokenError < StandardError; end
+
   attr_reader :user
 
   # Git clients will not know what authenticity token to send along
@@ -21,18 +23,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
     if allow_basic_auth? && basic_auth_provided?
       login, password = user_name_and_password(request)
-      auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
-      if auth_result.type == :ci && download_request?
-        @ci = true
-      elsif auth_result.type == :oauth && !download_request?
-        # Not allowed
-      elsif auth_result.type == :missing_personal_token
-        render_missing_personal_token
-        return # Render above denied access, nothing left to do
-      else
-        @user = auth_result.user
-      end
+      handle_authentication(login, password)
 
       if ci? || user
         return # Allow access
@@ -48,6 +40,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
     send_challenges
     render plain: "HTTP Basic: Access denied\n", status: 401
+
+  rescue MissingPersonalTokenError
+    render_missing_personal_token
+    return
   end
 
   def basic_auth_provided?
@@ -118,6 +114,28 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @ci.present?
   end
 
+  def handle_authentication(login, password)
+    auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
+
+    if auth_result.type == :ci && download_request?
+      @ci = true
+    elsif auth_result.type == :oauth && !download_request?
+      # Not allowed
+    elsif auth_result.type == :missing_personal_token
+      raise MissingPersonalTokenError
+    elsif auth_result.type == :lfs_deploy_token && download_request?
+      @lfs_deploy_key = true
+      @user = auth_result.user
+    else
+      @user = auth_result.user
+    end
+  end
+
+  def lfs_deploy_key?
+    key = user
+    @lfs_deploy_key.present? && (key && key.projects.include?(project))
+  end
+
   def verify_workhorse_api!
     Gitlab::Workhorse.verify_api_request!(request.headers)
   end
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 5d82abfca79..0c867fc8741 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,7 +25,7 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    project.public? || ci? || (user && user.can?(:download_code, project))
+    project.public? || ci? || lfs_deploy_key? || (user && user.can?(:download_code, project))
   end
 
   def lfs_upload_access?
diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb
index 2c525d4cd7a..de51b63c120 100644
--- a/app/models/deploy_key.rb
+++ b/app/models/deploy_key.rb
@@ -1,7 +1,12 @@
 class DeployKey < Key
+  include TokenAuthenticatable
+  add_authentication_token_field :lfs_token
+
   has_many :deploy_keys_projects, dependent: :destroy
   has_many :projects, through: :deploy_keys_projects
 
+  before_save :ensure_lfs_token
+
   scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
   scope :are_public,  -> { where(public: true) }
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 6996740eebd..94ae3911859 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -13,6 +13,7 @@ class User < ActiveRecord::Base
   DEFAULT_NOTIFICATION_LEVEL = :participating
 
   add_authentication_token_field :authentication_token
+  add_authentication_token_field :lfs_token
 
   default_value_for :admin, false
   default_value_for(:external) { current_application_settings.user_default_external }
@@ -117,7 +118,7 @@ class User < ActiveRecord::Base
   before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
 
   after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? }
-  before_save :ensure_authentication_token
+  before_save :ensure_authentication_token, :ensure_lfs_token
   before_save :ensure_external_user_rights
   after_save :ensure_namespace_correct
   after_initialize :set_projects_limit
diff --git a/db/migrate/20160825173042_add_lfs_token_to_users.rb b/db/migrate/20160825173042_add_lfs_token_to_users.rb
new file mode 100644
index 00000000000..f7f210bcd67
--- /dev/null
+++ b/db/migrate/20160825173042_add_lfs_token_to_users.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLfsTokenToUsers < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def change
+    add_column :users, :lfs_token, :string
+    add_concurrent_index :users, :lfs_token
+  end
+end
diff --git a/db/migrate/20160825182924_add_lfs_token_to_keys.rb b/db/migrate/20160825182924_add_lfs_token_to_keys.rb
new file mode 100644
index 00000000000..3ff010ef328
--- /dev/null
+++ b/db/migrate/20160825182924_add_lfs_token_to_keys.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLfsTokenToKeys < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def change
+    add_column :keys, :lfs_token, :string
+    add_concurrent_index :keys, :lfs_token
+  end
+end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 4f736e4ec2b..b4fcacca896 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1,7 +1,7 @@
 module API
   module Entities
     class UserSafe < Grape::Entity
-      expose :name, :username
+      expose :name, :username, :lfs_token
     end
 
     class UserBasic < UserSafe
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 6e6efece7c4..7c0a6eaa652 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -69,6 +69,10 @@ module API
             else
               project.repository.path_to_repo
             end
+
+          # Return HTTP full path, so that gitlab-shell has this information
+          # ready for git-lfs-authenticate
+          response[:repository_http_path] = project.http_url_to_repo
         end
 
         response
@@ -83,7 +87,14 @@ module API
       #
       get "/discover" do
         key = Key.find(params[:key_id])
-        present key.user, with: Entities::UserSafe
+        user = key.user
+        if user
+          user.ensure_lfs_token!
+          present user, with: Entities::UserSafe
+        else
+          key.ensure_lfs_token!
+          { username: 'lfs-deploy-key', lfs_token: key.lfs_token }
+        end
       end
 
       get "/check" do
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 91f0270818a..5446093de4d 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -79,12 +79,13 @@ module Gitlab
         result =
           user_with_password_for_git(login, password) ||
           oauth_access_token_check(login, password) ||
+          lfs_token_check(login, password) ||
           personal_access_token_check(login, password)
 
         if result
           result.type = nil unless result.user
 
-          if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
+          if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled?
             result.type = :missing_personal_token
           end
         end
@@ -114,6 +115,16 @@ module Gitlab
           Result.new(user, :personal_token) if user == validation
         end
       end
+
+      def lfs_token_check(login, password)
+        if login == 'lfs-deploy-key'
+          key = DeployKey.find_by_lfs_token(password)
+          Result.new(key, :lfs_deploy_token) if key
+        else
+          user = User.find_by_lfs_token(password)
+          Result.new(user, :lfs_token) if user && user.username == login
+        end
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 7c23e02d05a..cd00a15be3b 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -23,6 +23,22 @@ describe Gitlab::Auth, lib: true do
       expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
     end
 
+    it 'recognizes user lfs tokens' do
+      user = create(:user)
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
+      expect(gl_auth.find_for_git_client(user.username, user.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token))
+    end
+
+    it 'recognizes deploy key lfs tokens' do
+      key = create(:deploy_key)
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'lfs-deploy-key')
+      expect(gl_auth.find_for_git_client('lfs-deploy-key', key.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
+    end
+
     it 'recognizes OAuth tokens' do
       user = create(:user)
       application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index eb64f3d0c83..82076600f3b 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -18,6 +18,26 @@ describe User, 'TokenAuthenticatable' do
     subject { create(:user).send(token_field) }
     it { is_expected.to be_a String }
   end
+
+  describe 'lfs token' do
+    let(:token_field) { :lfs_token }
+    it_behaves_like 'TokenAuthenticatable'
+
+    describe 'ensure it' do
+      subject { create(:user).send(token_field) }
+      it { is_expected.to be_a String }
+    end
+  end
+end
+
+describe DeployKey, 'TokenAuthenticatable' do
+  let(:token_field) { :lfs_token }
+  it_behaves_like 'TokenAuthenticatable'
+
+  describe 'ensures authentication token' do
+    subject { create(:deploy_key).send(token_field) }
+    it { is_expected.to be_a String }
+  end
 end
 
 describe ApplicationSetting, 'TokenAuthenticatable' do
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 46d1b868782..95fc5f790e8 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -101,12 +101,28 @@ describe API::API, api: true  do
   end
 
   describe "GET /internal/discover" do
-    it do
-      get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+    context 'user key' do
+      it 'returns the correct information about the key' do
+        get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
 
-      expect(response).to have_http_status(200)
+        expect(response).to have_http_status(200)
 
-      expect(json_response['name']).to eq(user.name)
+        expect(json_response['name']).to eq(user.name)
+        expect(json_response['lfs_token']).to eq(user.lfs_token)
+      end
+    end
+
+    context 'deploy key' do
+      let(:key) { create(:deploy_key) }
+
+      it 'returns the correct information about the key' do
+        get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+
+        expect(response).to have_http_status(200)
+
+        expect(json_response['username']).to eq('lfs-deploy-key')
+        expect(json_response['lfs_token']).to eq(key.lfs_token)
+      end
     end
   end
 
@@ -143,6 +159,7 @@ describe API::API, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
+          expect(json_response["repository_http_path"]).to eq(project.http_url_to_repo)
         end
       end
 
@@ -153,6 +170,7 @@ describe API::API, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
+          expect(json_response["repository_http_path"]).to eq(project.http_url_to_repo)
         end
       end
     end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 6e551bb65fa..58f8515c0e2 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -244,6 +244,18 @@ describe 'Git LFS API and storage' do
           end
         end
 
+        context 'when deploy key is authorized' do
+          let(:key) { create(:deploy_key) }
+          let(:authorization) { authorize_deploy_key }
+
+          let(:update_permissions) do
+            project.deploy_keys << key
+            project.lfs_objects << lfs_object
+          end
+
+          it_behaves_like 'responds with a file'
+        end
+
         context 'when CI is authorized' do
           let(:authorization) { authorize_ci_project }
 
@@ -904,6 +916,10 @@ describe 'Git LFS API and storage' do
     ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
   end
 
+  def authorize_deploy_key
+    ActionController::HttpAuthentication::Basic.encode_credentials('lfs-deploy-key', key.lfs_token)
+  end
+
   def fork_project(project, user, object = nil)
     allow(RepositoryForkWorker).to receive(:perform_async).and_return(true)
     Projects::ForkService.new(project, user, {}).execute

From 372be2d2e8fe8d607011aa7e2b2fca99eeea007d Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 25 Aug 2016 18:43:14 -0500
Subject: [PATCH 14/41] Added CHANGELOG item and documentation.

---
 CHANGELOG                                              | 1 +
 doc/workflow/lfs/lfs_administration.md                 | 4 ++--
 doc/workflow/lfs/manage_large_binaries_with_git_lfs.md | 8 ++++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 0056c6cc649..9458413669d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -93,6 +93,7 @@ v 8.12.0 (unreleased)
   - Remove green outline from `New branch unavailable` button on issue page !5858 (winniehell)
   - Fix repo title alignment (ClemMakesApps)
   - Change update interval of contacted_at
+  - Add LFS support to SSH !6043
   - Fix branch title trailing space on hover (ClemMakesApps)
   - Don't include 'Created By' tag line when importing from GitHub if there is a linked GitLab account (EspadaV8)
   - Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison)
diff --git a/doc/workflow/lfs/lfs_administration.md b/doc/workflow/lfs/lfs_administration.md
index 9dc1e9b47e3..b3c73e947f0 100644
--- a/doc/workflow/lfs/lfs_administration.md
+++ b/doc/workflow/lfs/lfs_administration.md
@@ -45,5 +45,5 @@ In `config/gitlab.yml`:
 * Currently, storing GitLab Git LFS objects on a non-local storage (like S3 buckets)
   is not supported
 * Currently, removing LFS objects from GitLab Git LFS storage is not supported
-* LFS authentications via SSH is not supported for the time being
-* Only compatible with the GitLFS client versions 1.1.0 or 1.0.2.
+* LFS authentications via SSH was added with GitLab 8.12
+* Only compatible with the GitLFS client versions 1.1.0 and up, or 1.0.2.
diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
index 9fe065fa680..1a4f213a792 100644
--- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
+++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
@@ -35,6 +35,10 @@ Documentation for GitLab instance administrators is under [LFS administration do
   credentials store is recommended
 * Git LFS always assumes HTTPS so if you have GitLab server on HTTP you will have
   to add the URL to Git config manually (see #troubleshooting)
+  
+>**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication
+ still goes over HTTP, but now the SSH client passes the correct credentials
+ to the Git LFS client, so no action is required by the user.
 
 ## Using Git LFS
 
@@ -132,6 +136,10 @@ git config --add lfs.url "http://gitlab.example.com/group/project.git/info/lfs"
 
 ### Credentials are always required when pushing an object
 
+>**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication
+ still goes over HTTP, but now the SSH client passes the correct credentials
+ to the Git LFS client, so no action is required by the user.
+
 Given that Git LFS uses HTTP Basic Authentication to authenticate the user pushing
 the LFS object on every push for every object, user HTTPS credentials are required.
 

From cb85cf1f0a7047c485d7b29b2792b8965e270898 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Mon, 29 Aug 2016 13:05:07 -0500
Subject: [PATCH 15/41] Refactor LFS token logic to use a Redis key instead of
 a DB field, making it a 1 use only token.

---
 .../projects/git_http_client_controller.rb    |  3 +-
 app/helpers/lfs_helper.rb                     |  6 +++-
 app/models/deploy_key.rb                      |  5 ---
 app/models/user.rb                            |  3 +-
 .../20160825173042_add_lfs_token_to_users.rb  | 16 ---------
 .../20160825182924_add_lfs_token_to_keys.rb   | 16 ---------
 lib/api/entities.rb                           |  2 +-
 lib/api/internal.rb                           |  9 ++---
 lib/gitlab/auth.rb                            | 12 ++++---
 lib/gitlab/lfs_token.rb                       | 29 +++++++++++++++
 spec/lib/gitlab/auth_spec.rb                  |  8 +++--
 spec/lib/gitlab/lfs_token_spec.rb             | 35 +++++++++++++++++++
 .../concerns/token_authenticatable_spec.rb    | 20 -----------
 spec/requests/api/internal_spec.rb            |  6 ++--
 spec/requests/lfs_http_spec.rb                |  2 +-
 15 files changed, 93 insertions(+), 79 deletions(-)
 delete mode 100644 db/migrate/20160825173042_add_lfs_token_to_users.rb
 delete mode 100644 db/migrate/20160825182924_add_lfs_token_to_keys.rb
 create mode 100644 lib/gitlab/lfs_token.rb
 create mode 100644 spec/lib/gitlab/lfs_token_spec.rb

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 1e6709dc8eb..4dff1ce6568 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -132,8 +132,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
 
   def lfs_deploy_key?
-    key = user
-    @lfs_deploy_key.present? && (key && key.projects.include?(project))
+    @lfs_deploy_key.present? && (user && user.projects.include?(project))
   end
 
   def verify_workhorse_api!
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 0c867fc8741..2f5709a7395 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,7 +25,11 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    project.public? || ci? || lfs_deploy_key? || (user && user.can?(:download_code, project))
+    return true if project.public?
+    return true if ci?
+    return true if lfs_deploy_key?
+
+    (user && user.can?(:download_code, project))
   end
 
   def lfs_upload_access?
diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb
index de51b63c120..2c525d4cd7a 100644
--- a/app/models/deploy_key.rb
+++ b/app/models/deploy_key.rb
@@ -1,12 +1,7 @@
 class DeployKey < Key
-  include TokenAuthenticatable
-  add_authentication_token_field :lfs_token
-
   has_many :deploy_keys_projects, dependent: :destroy
   has_many :projects, through: :deploy_keys_projects
 
-  before_save :ensure_lfs_token
-
   scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
   scope :are_public,  -> { where(public: true) }
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 94ae3911859..6996740eebd 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -13,7 +13,6 @@ class User < ActiveRecord::Base
   DEFAULT_NOTIFICATION_LEVEL = :participating
 
   add_authentication_token_field :authentication_token
-  add_authentication_token_field :lfs_token
 
   default_value_for :admin, false
   default_value_for(:external) { current_application_settings.user_default_external }
@@ -118,7 +117,7 @@ class User < ActiveRecord::Base
   before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
 
   after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? }
-  before_save :ensure_authentication_token, :ensure_lfs_token
+  before_save :ensure_authentication_token
   before_save :ensure_external_user_rights
   after_save :ensure_namespace_correct
   after_initialize :set_projects_limit
diff --git a/db/migrate/20160825173042_add_lfs_token_to_users.rb b/db/migrate/20160825173042_add_lfs_token_to_users.rb
deleted file mode 100644
index f7f210bcd67..00000000000
--- a/db/migrate/20160825173042_add_lfs_token_to_users.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class AddLfsTokenToUsers < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  # Set this constant to true if this migration requires downtime.
-  DOWNTIME = false
-
-  disable_ddl_transaction!
-
-  def change
-    add_column :users, :lfs_token, :string
-    add_concurrent_index :users, :lfs_token
-  end
-end
diff --git a/db/migrate/20160825182924_add_lfs_token_to_keys.rb b/db/migrate/20160825182924_add_lfs_token_to_keys.rb
deleted file mode 100644
index 3ff010ef328..00000000000
--- a/db/migrate/20160825182924_add_lfs_token_to_keys.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class AddLfsTokenToKeys < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  # Set this constant to true if this migration requires downtime.
-  DOWNTIME = false
-
-  disable_ddl_transaction!
-
-  def change
-    add_column :keys, :lfs_token, :string
-    add_concurrent_index :keys, :lfs_token
-  end
-end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index b4fcacca896..4f736e4ec2b 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1,7 +1,7 @@
 module API
   module Entities
     class UserSafe < Grape::Entity
-      expose :name, :username, :lfs_token
+      expose :name, :username
     end
 
     class UserBasic < UserSafe
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 7c0a6eaa652..760f69663ab 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -88,12 +88,13 @@ module API
       get "/discover" do
         key = Key.find(params[:key_id])
         user = key.user
+
         if user
-          user.ensure_lfs_token!
-          present user, with: Entities::UserSafe
+          token = Gitlab::LfsToken.new(user).set_token
+          { name: user.name, username: user.username, lfs_token: token }
         else
-          key.ensure_lfs_token!
-          { username: 'lfs-deploy-key', lfs_token: key.lfs_token }
+          token = Gitlab::LfsToken.new(key).set_token
+          { username: "lfs-deploy-key-#{key.id}", lfs_token: token }
         end
       end
 
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 5446093de4d..e43f8119658 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -117,12 +117,14 @@ module Gitlab
       end
 
       def lfs_token_check(login, password)
-        if login == 'lfs-deploy-key'
-          key = DeployKey.find_by_lfs_token(password)
-          Result.new(key, :lfs_deploy_token) if key
+        if login.include?('lfs-deploy-key')
+          key = DeployKey.find(login.gsub('lfs-deploy-key-', ''))
+          token = Gitlab::LfsToken.new(key).get_value
+          Result.new(key, :lfs_deploy_token) if key && token == password
         else
-          user = User.find_by_lfs_token(password)
-          Result.new(user, :lfs_token) if user && user.username == login
+          user = User.by_login(login)
+          token = Gitlab::LfsToken.new(user).get_value
+          Result.new(user, :lfs_token) if user && token == password
         end
       end
     end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
new file mode 100644
index 00000000000..0685eb775ef
--- /dev/null
+++ b/lib/gitlab/lfs_token.rb
@@ -0,0 +1,29 @@
+module Gitlab
+  class LfsToken
+    attr_accessor :actor
+
+    def initialize(actor)
+      @actor = actor
+    end
+
+    def set_token
+      token = Devise.friendly_token(50)
+      Gitlab::Redis.with do |redis|
+        redis.set(redis_key, token, ex: 3600)
+      end
+      token
+    end
+
+    def get_value
+      Gitlab::Redis.with do |redis|
+        redis.get(redis_key)
+      end
+    end
+
+    private
+
+    def redis_key
+      "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor
+    end
+  end
+end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index cd00a15be3b..6ce680e3c26 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -26,17 +26,19 @@ describe Gitlab::Auth, lib: true do
     it 'recognizes user lfs tokens' do
       user = create(:user)
       ip = 'ip'
+      token = Gitlab::LfsToken.new(user).set_token
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find_for_git_client(user.username, user.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token))
+      expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token))
     end
 
     it 'recognizes deploy key lfs tokens' do
       key = create(:deploy_key)
       ip = 'ip'
+      token = Gitlab::LfsToken.new(key).set_token
 
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'lfs-deploy-key')
-      expect(gl_auth.find_for_git_client('lfs-deploy-key', key.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs-deploy-key-#{key.id}")
+      expect(gl_auth.find_for_git_client("lfs-deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
     end
 
     it 'recognizes OAuth tokens' do
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
new file mode 100644
index 00000000000..76b348637c7
--- /dev/null
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe Gitlab::LfsToken, lib: true do
+  describe '#set_token and #get_value' do
+    shared_examples 'an LFS token generator' do
+      it 'returns a randomly generated token' do
+        token = handler.set_token
+
+        expect(token).not_to be_nil
+        expect(token).to be_a String
+        expect(token.length).to eq 50
+      end
+
+      it 'returns the correct token based on the key' do
+        token = handler.set_token
+
+        expect(handler.get_value).to eq(token)
+      end
+    end
+
+    context 'when the actor is a user' do
+      let(:actor) { create(:user) }
+      let(:handler) { described_class.new(actor) }
+
+      it_behaves_like 'an LFS token generator'
+    end
+
+    context 'when the actor is a deploy key' do
+      let(:actor) { create(:deploy_key) }
+      let(:handler) { described_class.new(actor) }
+
+      it_behaves_like 'an LFS token generator'
+    end
+  end
+end
diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index 82076600f3b..eb64f3d0c83 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -18,26 +18,6 @@ describe User, 'TokenAuthenticatable' do
     subject { create(:user).send(token_field) }
     it { is_expected.to be_a String }
   end
-
-  describe 'lfs token' do
-    let(:token_field) { :lfs_token }
-    it_behaves_like 'TokenAuthenticatable'
-
-    describe 'ensure it' do
-      subject { create(:user).send(token_field) }
-      it { is_expected.to be_a String }
-    end
-  end
-end
-
-describe DeployKey, 'TokenAuthenticatable' do
-  let(:token_field) { :lfs_token }
-  it_behaves_like 'TokenAuthenticatable'
-
-  describe 'ensures authentication token' do
-    subject { create(:deploy_key).send(token_field) }
-    it { is_expected.to be_a String }
-  end
 end
 
 describe ApplicationSetting, 'TokenAuthenticatable' do
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 95fc5f790e8..59df5af770b 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -108,7 +108,7 @@ describe API::API, api: true  do
         expect(response).to have_http_status(200)
 
         expect(json_response['name']).to eq(user.name)
-        expect(json_response['lfs_token']).to eq(user.lfs_token)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).get_value)
       end
     end
 
@@ -120,8 +120,8 @@ describe API::API, api: true  do
 
         expect(response).to have_http_status(200)
 
-        expect(json_response['username']).to eq('lfs-deploy-key')
-        expect(json_response['lfs_token']).to eq(key.lfs_token)
+        expect(json_response['username']).to eq("lfs-deploy-key-#{key.id}")
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).get_value)
       end
     end
   end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 58f8515c0e2..d15e72b2570 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -917,7 +917,7 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_deploy_key
-    ActionController::HttpAuthentication::Basic.encode_credentials('lfs-deploy-key', key.lfs_token)
+    ActionController::HttpAuthentication::Basic.encode_credentials("lfs-deploy-key-#{key.id}", Gitlab::LfsToken.new(key).set_token)
   end
 
   def fork_project(project, user, object = nil)

From 48f1a61fd5c6aac395be0ce5d59aee61bbb69fe9 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Tue, 30 Aug 2016 13:38:22 -0500
Subject: [PATCH 16/41] Refactored LFS auth logic when using SSH to use its own
 API endpoint `/lfs_authenticate` and added tests.

---
 lib/api/internal.rb                | 32 ++++++++++++++----------
 lib/gitlab/auth.rb                 |  4 +--
 lib/gitlab/lfs_token.rb            |  8 +++---
 spec/lib/gitlab/auth_spec.rb       |  4 +--
 spec/lib/gitlab/lfs_token_spec.rb  |  6 ++---
 spec/requests/api/internal_spec.rb | 40 +++++++++++++++++++++++-------
 spec/requests/lfs_http_spec.rb     |  2 +-
 7 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 760f69663ab..1b3388347a8 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -69,15 +69,29 @@ module API
             else
               project.repository.path_to_repo
             end
-
-          # Return HTTP full path, so that gitlab-shell has this information
-          # ready for git-lfs-authenticate
-          response[:repository_http_path] = project.http_url_to_repo
         end
 
         response
       end
 
+      post "/lfs_authenticate" do
+        status 200
+
+        key = Key.find(params[:key_id])
+        user = key.user
+
+        if user
+          token = Gitlab::LfsToken.new(user).generate
+          response = { username: user.username, lfs_token: token }
+        else
+          token = Gitlab::LfsToken.new(key).generate
+          response = { username: "lfs-deploy-key-#{key.id}", lfs_token: token }
+        end
+
+        response[:repository_http_path] = project.http_url_to_repo
+        response
+      end
+
       get "/merge_request_urls" do
         ::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
       end
@@ -87,15 +101,7 @@ module API
       #
       get "/discover" do
         key = Key.find(params[:key_id])
-        user = key.user
-
-        if user
-          token = Gitlab::LfsToken.new(user).set_token
-          { name: user.name, username: user.username, lfs_token: token }
-        else
-          token = Gitlab::LfsToken.new(key).set_token
-          { username: "lfs-deploy-key-#{key.id}", lfs_token: token }
-        end
+        present key.user, with: Entities::UserSafe
       end
 
       get "/check" do
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index e43f8119658..1b0398d18ee 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -119,11 +119,11 @@ module Gitlab
       def lfs_token_check(login, password)
         if login.include?('lfs-deploy-key')
           key = DeployKey.find(login.gsub('lfs-deploy-key-', ''))
-          token = Gitlab::LfsToken.new(key).get_value
+          token = Gitlab::LfsToken.new(key).value
           Result.new(key, :lfs_deploy_token) if key && token == password
         else
           user = User.by_login(login)
-          token = Gitlab::LfsToken.new(user).get_value
+          token = Gitlab::LfsToken.new(user).value
           Result.new(user, :lfs_token) if user && token == password
         end
       end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 0685eb775ef..63656f0b4f1 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -6,15 +6,17 @@ module Gitlab
       @actor = actor
     end
 
-    def set_token
+    def generate
       token = Devise.friendly_token(50)
+
       Gitlab::Redis.with do |redis|
-        redis.set(redis_key, token, ex: 3600)
+        redis.set(redis_key, token, ex: 600)
       end
+
       token
     end
 
-    def get_value
+    def value
       Gitlab::Redis.with do |redis|
         redis.get(redis_key)
       end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 6ce680e3c26..4c8e09cd904 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Auth, lib: true do
     it 'recognizes user lfs tokens' do
       user = create(:user)
       ip = 'ip'
-      token = Gitlab::LfsToken.new(user).set_token
+      token = Gitlab::LfsToken.new(user).generate
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
       expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token))
@@ -35,7 +35,7 @@ describe Gitlab::Auth, lib: true do
     it 'recognizes deploy key lfs tokens' do
       key = create(:deploy_key)
       ip = 'ip'
-      token = Gitlab::LfsToken.new(key).set_token
+      token = Gitlab::LfsToken.new(key).generate
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs-deploy-key-#{key.id}")
       expect(gl_auth.find_for_git_client("lfs-deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 76b348637c7..1d2e4fd9566 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -4,7 +4,7 @@ describe Gitlab::LfsToken, lib: true do
   describe '#set_token and #get_value' do
     shared_examples 'an LFS token generator' do
       it 'returns a randomly generated token' do
-        token = handler.set_token
+        token = handler.generate
 
         expect(token).not_to be_nil
         expect(token).to be_a String
@@ -12,9 +12,9 @@ describe Gitlab::LfsToken, lib: true do
       end
 
       it 'returns the correct token based on the key' do
-        token = handler.set_token
+        token = handler.generate
 
-        expect(handler.get_value).to eq(token)
+        expect(handler.value).to eq(token)
       end
     end
 
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 59df5af770b..ff697286927 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -100,15 +100,20 @@ describe API::API, api: true  do
     end
   end
 
-  describe "GET /internal/discover" do
+  describe "POST /internal/lfs_authenticate" do
+    before do
+      project.team << [user, :developer]
+    end
+
     context 'user key' do
       it 'returns the correct information about the key' do
-        get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+        lfs_auth(key, project)
 
         expect(response).to have_http_status(200)
+        expect(json_response['username']).to eq(user.username)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).value)
 
-        expect(json_response['name']).to eq(user.name)
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).get_value)
+        expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
     end
 
@@ -116,16 +121,26 @@ describe API::API, api: true  do
       let(:key) { create(:deploy_key) }
 
       it 'returns the correct information about the key' do
-        get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+        lfs_auth(key, project)
 
         expect(response).to have_http_status(200)
-
         expect(json_response['username']).to eq("lfs-deploy-key-#{key.id}")
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).get_value)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
+        expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
     end
   end
 
+  describe "GET /internal/discover" do
+    it do
+      get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+
+      expect(response).to have_http_status(200)
+
+      expect(json_response['name']).to eq(user.name)
+    end
+  end
+
   describe "POST /internal/allowed" do
     context "access granted" do
       before do
@@ -159,7 +174,6 @@ describe API::API, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
-          expect(json_response["repository_http_path"]).to eq(project.http_url_to_repo)
         end
       end
 
@@ -170,7 +184,6 @@ describe API::API, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
-          expect(json_response["repository_http_path"]).to eq(project.http_url_to_repo)
         end
       end
     end
@@ -407,4 +420,13 @@ describe API::API, api: true  do
       protocol: 'ssh'
     )
   end
+
+  def lfs_auth(key, project)
+    post(
+      api("/internal/lfs_authenticate"),
+      key_id: key.id,
+      secret_token: secret_token,
+      project: project.path_with_namespace
+    )
+  end
 end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index d15e72b2570..e61502400ff 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -917,7 +917,7 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_deploy_key
-    ActionController::HttpAuthentication::Basic.encode_credentials("lfs-deploy-key-#{key.id}", Gitlab::LfsToken.new(key).set_token)
+    ActionController::HttpAuthentication::Basic.encode_credentials("lfs-deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
   end
 
   def fork_project(project, user, object = nil)

From c25630ee2c2804e351a2c3ae4fd9224434e4698a Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Tue, 30 Aug 2016 18:43:24 -0500
Subject: [PATCH 17/41] Refactored handling of the `LfsToken` and added
 functionality to it to simplify external code.

---
 app/helpers/lfs_helper.rb         |  4 +---
 lib/api/internal.rb               | 20 +++++++++++---------
 lib/gitlab/auth.rb                | 19 ++++++++++---------
 lib/gitlab/lfs_token.rb           |  8 ++++++++
 spec/lib/gitlab/lfs_token_spec.rb | 16 ++++++++++++++++
 5 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 2f5709a7395..031e7e72909 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,9 +25,7 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    return true if project.public?
-    return true if ci?
-    return true if lfs_deploy_key?
+    return true if project.public? || ci? || lfs_deploy_key?
 
     (user && user.can?(:download_code, project))
   end
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 1b3388347a8..1f189d81d16 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -80,16 +80,18 @@ module API
         key = Key.find(params[:key_id])
         user = key.user
 
-        if user
-          token = Gitlab::LfsToken.new(user).generate
-          response = { username: user.username, lfs_token: token }
-        else
-          token = Gitlab::LfsToken.new(key).generate
-          response = { username: "lfs-deploy-key-#{key.id}", lfs_token: token }
-        end
+        token_handler =
+          if user
+            Gitlab::LfsToken.new(user)
+          else
+            Gitlab::LfsToken.new(key)
+          end
 
-        response[:repository_http_path] = project.http_url_to_repo
-        response
+        {
+          username: token_handler.actor_name,
+          lfs_token: token_handler.generate,
+          repository_http_path: project.http_url_to_repo
+        }
       end
 
       get "/merge_request_urls" do
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 1b0398d18ee..821c0ef87e9 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -117,15 +117,16 @@ module Gitlab
       end
 
       def lfs_token_check(login, password)
-        if login.include?('lfs-deploy-key')
-          key = DeployKey.find(login.gsub('lfs-deploy-key-', ''))
-          token = Gitlab::LfsToken.new(key).value
-          Result.new(key, :lfs_deploy_token) if key && token == password
-        else
-          user = User.by_login(login)
-          token = Gitlab::LfsToken.new(user).value
-          Result.new(user, :lfs_token) if user && token == password
-        end
+        actor =
+          if login.include?('lfs-deploy-key')
+            DeployKey.find(login.gsub('lfs-deploy-key-', ''))
+          else
+            User.by_login(login)
+          end
+
+        token_handler = Gitlab::LfsToken.new(actor)
+
+        Result.new(actor, token_handler.type) if actor && token_handler.value == password
       end
     end
   end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 63656f0b4f1..8f49deb4d03 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -22,6 +22,14 @@ module Gitlab
       end
     end
 
+    def type
+      actor.is_a?(User) ? :lfs_token : :lfs_deploy_token
+    end
+
+    def actor_name
+      actor.is_a?(User) ? actor.username : "lfs-deploy-key-#{actor.id}"
+    end
+
     private
 
     def redis_key
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 1d2e4fd9566..f9812664e3b 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -23,6 +23,14 @@ describe Gitlab::LfsToken, lib: true do
       let(:handler) { described_class.new(actor) }
 
       it_behaves_like 'an LFS token generator'
+
+      it 'returns the correct username' do
+        expect(handler.actor_name).to eq(actor.username)
+      end
+
+      it 'returns the correct token type' do
+        expect(handler.type).to eq(:lfs_token)
+      end
     end
 
     context 'when the actor is a deploy key' do
@@ -30,6 +38,14 @@ describe Gitlab::LfsToken, lib: true do
       let(:handler) { described_class.new(actor) }
 
       it_behaves_like 'an LFS token generator'
+
+      it 'returns the correct username' do
+        expect(handler.actor_name).to eq("lfs-deploy-key-#{actor.id}")
+      end
+
+      it 'returns the correct token type' do
+        expect(handler.type).to eq(:lfs_deploy_token)
+      end
     end
   end
 end

From 85152f0291b7e6dd4a92a068e7d5c4334df54e80 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 31 Aug 2016 11:03:46 -0500
Subject: [PATCH 18/41] Improve string handling.

---
 lib/gitlab/auth.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 821c0ef87e9..02b33c8c683 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -118,8 +118,8 @@ module Gitlab
 
       def lfs_token_check(login, password)
         actor =
-          if login.include?('lfs-deploy-key')
-            DeployKey.find(login.gsub('lfs-deploy-key-', ''))
+          if login.start_with?('lfs-deploy-key')
+            DeployKey.find(login.sub('lfs-deploy-key-', ''))
           else
             User.by_login(login)
           end

From c144db2935f0f71c7f282a3015d126526bc16b57 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Tue, 6 Sep 2016 16:32:39 -0500
Subject: [PATCH 19/41] Better authentication handling, syntax fixes and better
 actor handling for LFS Tokens

---
 .../projects/git_http_client_controller.rb    | 27 +++++++-------
 app/helpers/lfs_helper.rb                     |  2 +-
 lib/api/internal.rb                           |  9 +----
 lib/gitlab/auth.rb                            | 35 +++++++++----------
 lib/gitlab/lfs_token.rb                       | 21 +++++++++--
 spec/requests/api/internal_spec.rb            |  2 +-
 6 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 4dff1ce6568..b4ec5b3fae1 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,8 +4,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  class MissingPersonalTokenError < StandardError; end
-
   attr_reader :user
 
   # Git clients will not know what authenticity token to send along
@@ -40,10 +38,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
     send_challenges
     render plain: "HTTP Basic: Access denied\n", status: 401
-
-  rescue MissingPersonalTokenError
+  rescue Gitlab::Auth::MissingPersonalTokenError
     render_missing_personal_token
-    return
   end
 
   def basic_auth_provided?
@@ -117,17 +113,20 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   def handle_authentication(login, password)
     auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
-    if auth_result.type == :ci && download_request?
-      @ci = true
-    elsif auth_result.type == :oauth && !download_request?
-      # Not allowed
-    elsif auth_result.type == :missing_personal_token
-      raise MissingPersonalTokenError
-    elsif auth_result.type == :lfs_deploy_token && download_request?
-      @lfs_deploy_key = true
+    case auth_result.type
+    when :ci
+      @ci = true if download_request?
+    when :oauth
+      @user = auth_result.user if download_request?
+    when :lfs_deploy_token
+      if download_request?
+        @lfs_deploy_key = true
+        @user = auth_result.user
+      end
+    when :lfs_token, :personal_token, :gitlab_or_ldap
       @user = auth_result.user
     else
-      @user = auth_result.user
+      # Not allowed
     end
   end
 
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 031e7e72909..de7c9f253b2 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -27,7 +27,7 @@ module LfsHelper
 
     return true if project.public? || ci? || lfs_deploy_key?
 
-    (user && user.can?(:download_code, project))
+    user && user.can?(:download_code, project)
   end
 
   def lfs_upload_access?
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 1f189d81d16..f8211bdd8af 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -78,14 +78,7 @@ module API
         status 200
 
         key = Key.find(params[:key_id])
-        user = key.user
-
-        token_handler =
-          if user
-            Gitlab::LfsToken.new(user)
-          else
-            Gitlab::LfsToken.new(key)
-          end
+        token_handler = Gitlab::LfsToken.new(key)
 
         {
           username: token_handler.actor_name,
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 02b33c8c683..14e29124aac 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -2,21 +2,13 @@ module Gitlab
   module Auth
     Result = Struct.new(:user, :type)
 
+    class MissingPersonalTokenError < StandardError; end
+
     class << self
       def find_for_git_client(login, password, project:, ip:)
         raise "Must provide an IP for rate limiting" if ip.nil?
 
-        result = Result.new
-
-        if valid_ci_request?(login, password, project)
-          result.type = :ci
-        else
-          result = populate_result(login, password)
-        end
-
-        success = result.user.present? || [:ci, :missing_personal_token].include?(result.type)
-        rate_limit!(ip, success: success, login: login)
-        result
+        populate_result(login, password, project, ip)
       end
 
       def find_with_user_password(login, password)
@@ -75,21 +67,26 @@ module Gitlab
         end
       end
 
-      def populate_result(login, password)
-        result =
+      def populate_result(login, password, project, ip)
+        result = Result.new(nil, :ci) if valid_ci_request?(login, password, project)
+
+        result ||=
           user_with_password_for_git(login, password) ||
           oauth_access_token_check(login, password) ||
           lfs_token_check(login, password) ||
           personal_access_token_check(login, password)
 
-        if result
+        if result && result.type != :ci
           result.type = nil unless result.user
 
           if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled?
-            result.type = :missing_personal_token
+            raise Gitlab::Auth::MissingPersonalTokenError
           end
         end
 
+        success = result ? result.user.present? || [:ci].include?(result.type) : false
+        rate_limit!(ip, success: success, login: login)
+
         result || Result.new
       end
 
@@ -118,15 +115,17 @@ module Gitlab
 
       def lfs_token_check(login, password)
         actor =
-          if login.start_with?('lfs-deploy-key')
-            DeployKey.find(login.sub('lfs-deploy-key-', ''))
+          if login =~ /\Alfs-deploy-key-\d+\Z/
+            /\d+\Z/.match(login) do |id|
+              DeployKey.find(id[0])
+            end
           else
             User.by_login(login)
           end
 
         token_handler = Gitlab::LfsToken.new(actor)
 
-        Result.new(actor, token_handler.type) if actor && token_handler.value == password
+        Result.new(actor, token_handler.type) if actor && Devise.secure_compare(token_handler.value, password)
       end
     end
   end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 8f49deb4d03..d7db8017475 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -2,15 +2,18 @@ module Gitlab
   class LfsToken
     attr_accessor :actor
 
+    TOKEN_LENGTH = 50
+    EXPIRY_TIME = 1800
+
     def initialize(actor)
-      @actor = actor
+      set_actor(actor)
     end
 
     def generate
-      token = Devise.friendly_token(50)
+      token = Devise.friendly_token(TOKEN_LENGTH)
 
       Gitlab::Redis.with do |redis|
-        redis.set(redis_key, token, ex: 600)
+        redis.set(redis_key, token, ex: EXPIRY_TIME)
       end
 
       token
@@ -35,5 +38,17 @@ module Gitlab
     def redis_key
       "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor
     end
+
+    def set_actor(actor)
+      @actor =
+        case actor
+        when DeployKey, User
+          actor
+        when Key
+          actor.user
+        else
+          #
+        end
+    end
   end
 end
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index ff697286927..1ee390e0a19 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -111,7 +111,7 @@ describe API::API, api: true  do
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq(user.username)
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).value)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
 
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end

From 71aff7f6a3ab63f1395bfab6ea49f0175fe08167 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 7 Sep 2016 11:55:54 -0500
Subject: [PATCH 20/41] Use special characters for `lfs+deploy-key` to prevent
 a someone from creating a user with this username, and method name
 refactoring.

---
 app/controllers/projects/git_http_client_controller.rb | 4 ++--
 lib/gitlab/auth.rb                                     | 2 +-
 lib/gitlab/lfs_token.rb                                | 2 +-
 spec/lib/gitlab/auth_spec.rb                           | 4 ++--
 spec/lib/gitlab/lfs_token_spec.rb                      | 2 +-
 spec/requests/api/internal_spec.rb                     | 2 +-
 spec/requests/lfs_http_spec.rb                         | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index b4ec5b3fae1..4be580e759e 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -22,7 +22,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     if allow_basic_auth? && basic_auth_provided?
       login, password = user_name_and_password(request)
 
-      handle_authentication(login, password)
+      handle_basic_authentication(login, password)
 
       if ci? || user
         return # Allow access
@@ -110,7 +110,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @ci.present?
   end
 
-  def handle_authentication(login, password)
+  def handle_basic_authentication(login, password)
     auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
     case auth_result.type
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 14e29124aac..f4e6ebb7bc7 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -115,7 +115,7 @@ module Gitlab
 
       def lfs_token_check(login, password)
         actor =
-          if login =~ /\Alfs-deploy-key-\d+\Z/
+          if login =~ /\Alfs\+deploy-key-\d+\Z/
             /\d+\Z/.match(login) do |id|
               DeployKey.find(id[0])
             end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index d7db8017475..edf4dffc4c0 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -30,7 +30,7 @@ module Gitlab
     end
 
     def actor_name
-      actor.is_a?(User) ? actor.username : "lfs-deploy-key-#{actor.id}"
+      actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}"
     end
 
     private
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 4c8e09cd904..56f349f5d92 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -37,8 +37,8 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
       token = Gitlab::LfsToken.new(key).generate
 
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs-deploy-key-#{key.id}")
-      expect(gl_auth.find_for_git_client("lfs-deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}")
+      expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
     end
 
     it 'recognizes OAuth tokens' do
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index f9812664e3b..184f235c1b2 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -40,7 +40,7 @@ describe Gitlab::LfsToken, lib: true do
       it_behaves_like 'an LFS token generator'
 
       it 'returns the correct username' do
-        expect(handler.actor_name).to eq("lfs-deploy-key-#{actor.id}")
+        expect(handler.actor_name).to eq("lfs+deploy-key-#{actor.id}")
       end
 
       it 'returns the correct token type' do
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 1ee390e0a19..2e1e6a11b53 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -124,7 +124,7 @@ describe API::API, api: true  do
         lfs_auth(key, project)
 
         expect(response).to have_http_status(200)
-        expect(json_response['username']).to eq("lfs-deploy-key-#{key.id}")
+        expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
         expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index e61502400ff..54ecb793729 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -917,7 +917,7 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_deploy_key
-    ActionController::HttpAuthentication::Basic.encode_credentials("lfs-deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
+    ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
   end
 
   def fork_project(project, user, object = nil)

From be09bcf074e6048aa9ba5f8dfb99754e6afbe156 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 15 Sep 2016 11:54:24 -0500
Subject: [PATCH 21/41] Refactored authentication code to make it a bit
 clearer, added test for wrong SSH key.

---
 .../projects/git_http_client_controller.rb    | 25 +++++++----
 lib/gitlab/auth.rb                            | 43 +++++++++----------
 lib/gitlab/lfs_token.rb                       |  2 +-
 spec/lib/gitlab/auth_spec.rb                  |  2 +-
 spec/lib/gitlab/lfs_token_spec.rb             |  2 +-
 spec/requests/api/internal_spec.rb            | 14 ++++--
 6 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index f5a07608bf8..4dae953b69f 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :user, :actor
+  attr_reader :actor
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -22,9 +22,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     if allow_basic_auth? && basic_auth_provided?
       login, password = user_name_and_password(request)
 
-      handle_basic_authentication(login, password)
-
-      if ci? || actor
+      if handle_basic_authentication(login, password)
         return # Allow access
       end
     elsif allow_kerberos_spnego_auth? && spnego_provided?
@@ -107,7 +105,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
 
   def ci?
-    @ci.present?
+    @ci
   end
 
   def user
@@ -119,9 +117,17 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
     case auth_result.type
     when :ci
-      @ci = true if download_request?
+      if download_request?
+        @ci = true
+      else
+        return false
+      end
     when :oauth
-      @actor = auth_result.actor if download_request?
+      if download_request?
+        @actor = auth_result.actor
+      else
+        return false
+      end
     when :lfs_deploy_token
       if download_request?
         @lfs_deploy_key = true
@@ -131,11 +137,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController
       @actor = auth_result.actor
     else
       # Not allowed
+      return false
     end
+
+    true
   end
 
   def lfs_deploy_key?
-    @lfs_deploy_key.present? && actor && actor.projects.include?(project)
+    @lfs_deploy_key && actor && actor.projects.include?(project)
   end
 
   def verify_workhorse_api!
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 391b8f2f5de..6be9bf7de44 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,10 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:actor, :type)
+    Result = Struct.new(:actor, :type) do
+      def success?
+        actor.present? || type == :ci
+      end
+    end
 
     class MissingPersonalTokenError < StandardError; end
 
@@ -8,7 +12,16 @@ module Gitlab
       def find_for_git_client(login, password, project:, ip:)
         raise "Must provide an IP for rate limiting" if ip.nil?
 
-        populate_result(login, password, project, ip)
+        result =
+          ci_request_check(login, password, project) ||
+          user_with_password_for_git(login, password) ||
+          oauth_access_token_check(login, password) ||
+          lfs_token_check(login, password) ||
+          personal_access_token_check(login, password)
+
+        rate_limit!(ip, success: result && result.success?, login: login)
+
+        result || Result.new
       end
 
       def find_with_user_password(login, password)
@@ -49,24 +62,6 @@ module Gitlab
 
       private
 
-      def populate_result(login, password, project, ip)
-        result =
-          ci_request_check(login, password, project) ||
-          user_with_password_for_git(login, password) ||
-          oauth_access_token_check(login, password) ||
-          lfs_token_check(login, password) ||
-          personal_access_token_check(login, password)
-
-        if result && result.type != :ci
-          result.type = nil unless result.actor
-        end
-
-        success = result ? result.actor.present? || result.type == :ci : false
-        rate_limit!(ip, success: success, login: login)
-
-        result || Result.new
-      end
-
       def valid_ci_request?(login, password, project)
         matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
 
@@ -110,7 +105,7 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, :personal_token) if user == validation
+          Result.new(user, :personal_token) if user.present? && user == validation
         end
       end
 
@@ -124,9 +119,11 @@ module Gitlab
             User.by_login(login)
           end
 
-        token_handler = Gitlab::LfsToken.new(actor)
+        if actor
+          token_handler = Gitlab::LfsToken.new(actor)
 
-        Result.new(actor, token_handler.type) if actor && Devise.secure_compare(token_handler.value, password)
+          Result.new(actor, token_handler.type) if Devise.secure_compare(token_handler.value, password)
+        end
       end
     end
   end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 224e4516074..f492754b1c8 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -13,7 +13,7 @@ module Gitlab
         when Key
           actor.user
         else
-          #
+          raise 'Bad Actor'
         end
     end
 
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 56f349f5d92..13c5a7156f5 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -55,7 +55,7 @@ describe Gitlab::Auth, lib: true do
       login = 'foo'
       ip = 'ip'
 
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: nil, login: login)
       expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
     end
   end
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 184f235c1b2..9f04f67e0a8 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::LfsToken, lib: true do
-  describe '#set_token and #get_value' do
+  describe '#generate and #value' do
     shared_examples 'an LFS token generator' do
       it 'returns a randomly generated token' do
         token = handler.generate
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 2e1e6a11b53..46e8e6f1169 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -107,7 +107,7 @@ describe API::API, api: true  do
 
     context 'user key' do
       it 'returns the correct information about the key' do
-        lfs_auth(key, project)
+        lfs_auth(key.id, project)
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq(user.username)
@@ -115,13 +115,19 @@ describe API::API, api: true  do
 
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
+
+      it 'returns a 404 when the wrong key is provided' do
+        lfs_auth(nil, project)
+
+        expect(response).to have_http_status(404)
+      end
     end
 
     context 'deploy key' do
       let(:key) { create(:deploy_key) }
 
       it 'returns the correct information about the key' do
-        lfs_auth(key, project)
+        lfs_auth(key.id, project)
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
@@ -421,10 +427,10 @@ describe API::API, api: true  do
     )
   end
 
-  def lfs_auth(key, project)
+  def lfs_auth(key_id, project)
     post(
       api("/internal/lfs_authenticate"),
-      key_id: key.id,
+      key_id: key_id,
       secret_token: secret_token,
       project: project.path_with_namespace
     )

From de24075ea5960bd7c6290c05496915e8f0ca23f2 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 8 Sep 2016 12:32:20 -0500
Subject: [PATCH 22/41] Further refactoring of authentication code, and code
 style fixes.

---
 .../projects/git_http_client_controller.rb    | 20 ++++---
 lib/gitlab/auth.rb                            | 53 ++++++++++---------
 lib/gitlab/lfs_token.rb                       | 22 ++++----
 3 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 4be580e759e..f5a07608bf8 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :user
+  attr_reader :user, :actor
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -24,13 +24,13 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
       handle_basic_authentication(login, password)
 
-      if ci? || user
+      if ci? || actor
         return # Allow access
       end
     elsif allow_kerberos_spnego_auth? && spnego_provided?
-      @user = find_kerberos_user
+      @actor = find_kerberos_user
 
-      if user
+      if actor
         send_final_spnego_response
         return # Allow access
       end
@@ -110,6 +110,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @ci.present?
   end
 
+  def user
+    @actor
+  end
+
   def handle_basic_authentication(login, password)
     auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
@@ -117,21 +121,21 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     when :ci
       @ci = true if download_request?
     when :oauth
-      @user = auth_result.user if download_request?
+      @actor = auth_result.actor if download_request?
     when :lfs_deploy_token
       if download_request?
         @lfs_deploy_key = true
-        @user = auth_result.user
+        @actor = auth_result.actor
       end
     when :lfs_token, :personal_token, :gitlab_or_ldap
-      @user = auth_result.user
+      @actor = auth_result.actor
     else
       # Not allowed
     end
   end
 
   def lfs_deploy_key?
-    @lfs_deploy_key.present? && (user && user.projects.include?(project))
+    @lfs_deploy_key.present? && actor && actor.projects.include?(project)
   end
 
   def verify_workhorse_api!
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index f4e6ebb7bc7..391b8f2f5de 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:user, :type)
+    Result = Struct.new(:actor, :type)
 
     class MissingPersonalTokenError < StandardError; end
 
@@ -49,6 +49,24 @@ module Gitlab
 
       private
 
+      def populate_result(login, password, project, ip)
+        result =
+          ci_request_check(login, password, project) ||
+          user_with_password_for_git(login, password) ||
+          oauth_access_token_check(login, password) ||
+          lfs_token_check(login, password) ||
+          personal_access_token_check(login, password)
+
+        if result && result.type != :ci
+          result.type = nil unless result.actor
+        end
+
+        success = result ? result.actor.present? || result.type == :ci : false
+        rate_limit!(ip, success: success, login: login)
+
+        result || Result.new
+      end
+
       def valid_ci_request?(login, password, project)
         matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
 
@@ -67,31 +85,14 @@ module Gitlab
         end
       end
 
-      def populate_result(login, password, project, ip)
-        result = Result.new(nil, :ci) if valid_ci_request?(login, password, project)
-
-        result ||=
-          user_with_password_for_git(login, password) ||
-          oauth_access_token_check(login, password) ||
-          lfs_token_check(login, password) ||
-          personal_access_token_check(login, password)
-
-        if result && result.type != :ci
-          result.type = nil unless result.user
-
-          if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled?
-            raise Gitlab::Auth::MissingPersonalTokenError
-          end
-        end
-
-        success = result ? result.user.present? || [:ci].include?(result.type) : false
-        rate_limit!(ip, success: success, login: login)
-
-        result || Result.new
+      def ci_request_check(login, password, project)
+        Result.new(nil, :ci) if valid_ci_request?(login, password, project)
       end
 
       def user_with_password_for_git(login, password)
         user = find_with_user_password(login, password)
+        raise Gitlab::Auth::MissingPersonalTokenError if user && user.two_factor_enabled?
+
         Result.new(user, :gitlab_or_ldap) if user
       end
 
@@ -114,11 +115,11 @@ module Gitlab
       end
 
       def lfs_token_check(login, password)
+        deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/)
+
         actor =
-          if login =~ /\Alfs\+deploy-key-\d+\Z/
-            /\d+\Z/.match(login) do |id|
-              DeployKey.find(id[0])
-            end
+          if deploy_key_matches
+            DeployKey.find(deploy_key_matches[1])
           else
             User.by_login(login)
           end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index edf4dffc4c0..224e4516074 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -6,7 +6,15 @@ module Gitlab
     EXPIRY_TIME = 1800
 
     def initialize(actor)
-      set_actor(actor)
+      @actor =
+        case actor
+        when DeployKey, User
+          actor
+        when Key
+          actor.user
+        else
+          #
+        end
     end
 
     def generate
@@ -38,17 +46,5 @@ module Gitlab
     def redis_key
       "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor
     end
-
-    def set_actor(actor)
-      @actor =
-        case actor
-        when DeployKey, User
-          actor
-        when Key
-          actor.user
-        else
-          #
-        end
-    end
   end
 end

From 5f45ddc54577fb65db00636a05408b00636544f5 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 22:17:12 +0200
Subject: [PATCH 23/41] Fix specs after merging LFS changes

---
 app/controllers/jwt_controller.rb    | 14 ++++++++++++--
 spec/lib/gitlab/auth_spec.rb         |  6 +++---
 spec/requests/jwt_controller_spec.rb | 24 +++++++++++++++++++++---
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 0870a2a8f50..a69534c2258 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -13,7 +13,7 @@ class JwtController < ApplicationController
 
     @authentication_result ||= Gitlab::Auth::Result.new
 
-    result = service.new(@authentication_result.project, @authentication_result.user, auth_params).
+    result = service.new(@authentication_result.project, @authentication_result.actor, auth_params).
       execute(capabilities: @authentication_result.capabilities)
 
     render json: result, status: result[:http_status]
@@ -25,8 +25,18 @@ class JwtController < ApplicationController
     authenticate_with_http_basic do |login, password|
       @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip)
 
-      render_403 unless @authentication_result.succeeded?
+      render_403 unless @authentication_result.success? &&
+        (@authentication_result.actor.nil? || @authentication_result.actor.is_a?(User))
     end
+  rescue Gitlab::Auth::MissingPersonalTokenError
+    render_missing_personal_token
+  end
+
+  def render_missing_personal_token
+    render plain: "HTTP Basic: Access denied\n" \
+                  "You have 2FA enabled, please use a personal access token for Git over HTTP.\n" \
+                  "You can generate one at #{profile_personal_access_tokens_url}",
+           status: 401
   end
 
   def auth_params
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index c09ab1dbd57..e24ad530904 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -65,7 +65,7 @@ describe Gitlab::Auth, lib: true do
       token = Gitlab::LfsToken.new(user).generate
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token))
+      expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, read_capabilities))
     end
 
     it 'recognizes deploy key lfs tokens' do
@@ -74,7 +74,7 @@ describe Gitlab::Auth, lib: true do
       token = Gitlab::LfsToken.new(key).generate
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}")
-      expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
+      expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_capabilities))
     end
 
     it 'recognizes OAuth tokens' do
@@ -91,7 +91,7 @@ describe Gitlab::Auth, lib: true do
       login = 'foo'
       ip = 'ip'
 
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: nil, login: login)
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
       expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
     end
   end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index 93b9cfaf33d..1ca4541dbde 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -45,13 +45,31 @@ describe JwtController do
 
     context 'using User login' do
       let(:user) { create(:user) }
-      let(:headers) { { authorization: credentials('user', 'password') } }
-
-      before { expect(Gitlab::Auth).to receive(:find_with_user_password).with('user', 'password').and_return(user) }
+      let(:headers) { { authorization: credentials(user.username  , user.password) } }
 
       subject! { get '/jwt/auth', parameters, headers }
 
       it { expect(service_class).to have_received(:new).with(nil, user, parameters) }
+
+      context 'when user has 2FA enabled' do
+        let(:user) { create(:user, :two_factor) }
+
+        context 'without personal token' do
+          it 'rejects the authorization attempt' do
+            expect(response).to have_http_status(401)
+            expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP')
+          end
+        end
+
+        context 'with personal token' do
+          let(:access_token) { create(:personal_access_token, user: user) }
+          let(:headers) { { authorization: credentials(user.username, access_token.token) } }
+
+          it 'rejects the authorization attempt' do
+            expect(response).to have_http_status(200)
+          end
+        end
+      end
     end
 
     context 'using invalid login' do

From ac6412d0766fbc090a3aa8272cfd4cc2d9a26c16 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 15 Sep 2016 23:27:01 +0200
Subject: [PATCH 24/41] Added builds_spec and git_http_specs

---
 .../projects/git_http_client_controller.rb    |  2 +-
 spec/requests/ci/api/builds_spec.rb           | 77 ++++++++++++++++---
 spec/requests/git_http_spec.rb                | 69 +++++++++++++++--
 spec/requests/jwt_controller_spec.rb          |  2 +-
 4 files changed, 130 insertions(+), 20 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 14e83ddda04..d92d28b7e02 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -117,7 +117,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
     case auth_result.type
     when :ci
-      if download_request?
+      if auth_result.project == project && download_request?
         @ci = true
       else
         return false
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index 780bd7f2859..09d72fe0a0e 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -254,7 +254,8 @@ describe Ci::API::API do
       let(:get_url) { ci_api("/builds/#{build.id}/artifacts") }
       let(:jwt_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
       let(:headers) { { "GitLab-Workhorse" => "1.0", Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => jwt_token } }
-      let(:headers_with_token) { headers.merge(Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token) }
+      let(:token) { build.token }
+      let(:headers_with_token) { headers.merge(Ci::API::Helpers::BUILD_TOKEN_HEADER => token) }
 
       before { build.run! }
 
@@ -274,6 +275,13 @@ describe Ci::API::API do
             expect(json_response["TempPath"]).not_to be_nil
           end
 
+          it "using runners token" do
+            post authorize_url, { token: build.project.runners_token }, headers
+            expect(response).to have_http_status(200)
+            expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+            expect(json_response["TempPath"]).not_to be_nil
+          end
+
           it "reject requests that did not go through gitlab-workhorse" do
             headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
             post authorize_url, { token: build.token }, headers
@@ -358,6 +366,16 @@ describe Ci::API::API do
 
               it_behaves_like 'successful artifacts upload'
             end
+
+            context 'when using runners token' do
+              let(:token) { build.project.runners_token }
+
+              before do
+                upload_artifacts(file_upload, headers_with_token)
+              end
+
+              it_behaves_like 'successful artifacts upload'
+            end
           end
 
           context 'posts artifacts file and metadata file' do
@@ -497,19 +515,40 @@ describe Ci::API::API do
 
         before do
           delete delete_url, token: build.token
-          build.reload
         end
 
-        it 'removes build artifacts' do
-          expect(response).to have_http_status(200)
-          expect(build.artifacts_file.exists?).to be_falsy
-          expect(build.artifacts_metadata.exists?).to be_falsy
-          expect(build.artifacts_size).to be_nil
+        shared_examples 'having removable artifacts' do
+          it 'removes build artifacts' do
+            build.reload
+
+            expect(response).to have_http_status(200)
+            expect(build.artifacts_file.exists?).to be_falsy
+            expect(build.artifacts_metadata.exists?).to be_falsy
+            expect(build.artifacts_size).to be_nil
+          end
+        end
+
+        context 'when using build token' do
+          before do
+            delete delete_url, token: build.token
+          end
+
+          it_behaves_like 'having removable artifacts'
+        end
+
+        context 'when using runnners token' do
+          before do
+            delete delete_url, token: build.project.runners_token
+          end
+
+          it_behaves_like 'having removable artifacts'
         end
       end
 
       describe 'GET /builds/:id/artifacts' do
-        before { get get_url, token: build.token }
+        before do
+          get get_url, token: token
+        end
 
         context 'build has artifacts' do
           let(:build) { create(:ci_build, :artifacts) }
@@ -518,13 +557,29 @@ describe Ci::API::API do
               'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
           end
 
-          it 'downloads artifact' do
-            expect(response).to have_http_status(200)
-            expect(response.headers).to include download_headers
+          shared_examples 'having downloadable artifacts' do
+            it 'download artifacts' do
+              expect(response).to have_http_status(200)
+              expect(response.headers).to include download_headers
+            end
+          end
+
+          context 'when using build token' do
+            let(:token) { build.token }
+
+            it_behaves_like 'having downloadable artifacts'
+          end
+
+          context 'when using runnners token' do
+            let(:token) { build.project.runners_token }
+
+            it_behaves_like 'having downloadable artifacts'
           end
         end
 
         context 'build does not has artifacts' do
+          let(:token) { build.token }
+
           it 'responds with not found' do
             expect(response).to have_http_status(404)
           end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 5977ee04524..0311755dd06 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -302,22 +302,77 @@ describe 'Git HTTP requests', lib: true do
       context "when a gitlab ci token is provided" do
         let(:build) { create(:ci_build, :running) }
         let(:project) { build.project }
+        let(:other_project) { create(:empty_project) }
 
         before do
           project.project_feature.update_attributes(builds_access_level: ProjectFeature::ENABLED)
         end
 
-        it "downloads get status 200" do
-          clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+        context 'when build created by system is authenticated' do
+          it "downloads get status 200" do
+            clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
-          expect(response).to have_http_status(200)
-          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+            expect(response).to have_http_status(200)
+            expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+          end
+
+          it "uploads get status 401 (no project existence information leak)" do
+            push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+
+            expect(response).to have_http_status(401)
+          end
+
+          it "downloads from other project get status 401" do
+            clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+
+            expect(response).to have_http_status(401)
+          end
         end
 
-        it "uploads get status 401 (no project existence information leak)" do
-          push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+        context 'and build created by' do
+          before do
+            build.update(user: user)
+            project.team << [user, :reporter]
+          end
 
-          expect(response).to have_http_status(401)
+          shared_examples 'can download code only from own projects' do
+            it 'downloads get status 200' do
+              clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+
+              expect(response).to have_http_status(200)
+              expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+            end
+
+            it 'uploads get status 403' do
+              push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+
+              expect(response).to have_http_status(403)
+            end
+          end
+
+          context 'administrator' do
+            let(:user) { create(:admin) }
+
+            it_behaves_like 'can download code only from own projects'
+
+            it 'downloads from other project get status 403' do
+              clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+
+              expect(response).to have_http_status(403)
+            end
+          end
+
+          context 'regular user' do
+            let(:user) { create(:user) }
+
+            it_behaves_like 'can download code only from own projects'
+
+            it 'downloads from other project get status 404' do
+              clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+
+              expect(response).to have_http_status(404)
+            end
+          end
         end
       end
     end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index 1ca4541dbde..6b956e63004 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -45,7 +45,7 @@ describe JwtController do
 
     context 'using User login' do
       let(:user) { create(:user) }
-      let(:headers) { { authorization: credentials(user.username  , user.password) } }
+      let(:headers) { { authorization: credentials(user.username, user.password) } }
 
       subject! { get '/jwt/auth', parameters, headers }
 

From 3d933082c36aad49829403d9d0c970d58860edb2 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 15 Sep 2016 11:54:24 -0500
Subject: [PATCH 25/41] Refactored authentication code to make it a bit
 clearer, added test for wrong SSH key.

---
 .../projects/git_http_client_controller.rb    | 27 ++++++++----
 lib/gitlab/auth.rb                            | 43 +++++++++----------
 lib/gitlab/lfs_token.rb                       |  2 +-
 spec/lib/gitlab/auth_spec.rb                  |  2 +-
 spec/lib/gitlab/lfs_token_spec.rb             |  2 +-
 spec/requests/api/internal_spec.rb            | 14 ++++--
 6 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index f5a07608bf8..f74a7000373 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :user, :actor
+  attr_reader :actor
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -22,9 +22,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     if allow_basic_auth? && basic_auth_provided?
       login, password = user_name_and_password(request)
 
-      handle_basic_authentication(login, password)
-
-      if ci? || actor
+      if handle_basic_authentication(login, password)
         return # Allow access
       end
     elsif allow_kerberos_spnego_auth? && spnego_provided?
@@ -107,7 +105,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
 
   def ci?
-    @ci.present?
+    @ci
   end
 
   def user
@@ -119,23 +117,36 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
     case auth_result.type
     when :ci
-      @ci = true if download_request?
+      if download_request?
+        @ci = true
+      else
+        return false
+      end
     when :oauth
-      @actor = auth_result.actor if download_request?
+      if download_request?
+        @actor = auth_result.actor
+      else
+        return false
+      end
     when :lfs_deploy_token
       if download_request?
         @lfs_deploy_key = true
         @actor = auth_result.actor
+      else
+        return false
       end
     when :lfs_token, :personal_token, :gitlab_or_ldap
       @actor = auth_result.actor
     else
       # Not allowed
+      return false
     end
+
+    true
   end
 
   def lfs_deploy_key?
-    @lfs_deploy_key.present? && actor && actor.projects.include?(project)
+    @lfs_deploy_key && actor && actor.projects.include?(project)
   end
 
   def verify_workhorse_api!
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 391b8f2f5de..6be9bf7de44 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,10 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:actor, :type)
+    Result = Struct.new(:actor, :type) do
+      def success?
+        actor.present? || type == :ci
+      end
+    end
 
     class MissingPersonalTokenError < StandardError; end
 
@@ -8,7 +12,16 @@ module Gitlab
       def find_for_git_client(login, password, project:, ip:)
         raise "Must provide an IP for rate limiting" if ip.nil?
 
-        populate_result(login, password, project, ip)
+        result =
+          ci_request_check(login, password, project) ||
+          user_with_password_for_git(login, password) ||
+          oauth_access_token_check(login, password) ||
+          lfs_token_check(login, password) ||
+          personal_access_token_check(login, password)
+
+        rate_limit!(ip, success: result && result.success?, login: login)
+
+        result || Result.new
       end
 
       def find_with_user_password(login, password)
@@ -49,24 +62,6 @@ module Gitlab
 
       private
 
-      def populate_result(login, password, project, ip)
-        result =
-          ci_request_check(login, password, project) ||
-          user_with_password_for_git(login, password) ||
-          oauth_access_token_check(login, password) ||
-          lfs_token_check(login, password) ||
-          personal_access_token_check(login, password)
-
-        if result && result.type != :ci
-          result.type = nil unless result.actor
-        end
-
-        success = result ? result.actor.present? || result.type == :ci : false
-        rate_limit!(ip, success: success, login: login)
-
-        result || Result.new
-      end
-
       def valid_ci_request?(login, password, project)
         matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
 
@@ -110,7 +105,7 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, :personal_token) if user == validation
+          Result.new(user, :personal_token) if user.present? && user == validation
         end
       end
 
@@ -124,9 +119,11 @@ module Gitlab
             User.by_login(login)
           end
 
-        token_handler = Gitlab::LfsToken.new(actor)
+        if actor
+          token_handler = Gitlab::LfsToken.new(actor)
 
-        Result.new(actor, token_handler.type) if actor && Devise.secure_compare(token_handler.value, password)
+          Result.new(actor, token_handler.type) if Devise.secure_compare(token_handler.value, password)
+        end
       end
     end
   end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 224e4516074..f492754b1c8 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -13,7 +13,7 @@ module Gitlab
         when Key
           actor.user
         else
-          #
+          raise 'Bad Actor'
         end
     end
 
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 56f349f5d92..13c5a7156f5 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -55,7 +55,7 @@ describe Gitlab::Auth, lib: true do
       login = 'foo'
       ip = 'ip'
 
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: nil, login: login)
       expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
     end
   end
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 184f235c1b2..9f04f67e0a8 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::LfsToken, lib: true do
-  describe '#set_token and #get_value' do
+  describe '#generate and #value' do
     shared_examples 'an LFS token generator' do
       it 'returns a randomly generated token' do
         token = handler.generate
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 2e1e6a11b53..46e8e6f1169 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -107,7 +107,7 @@ describe API::API, api: true  do
 
     context 'user key' do
       it 'returns the correct information about the key' do
-        lfs_auth(key, project)
+        lfs_auth(key.id, project)
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq(user.username)
@@ -115,13 +115,19 @@ describe API::API, api: true  do
 
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
+
+      it 'returns a 404 when the wrong key is provided' do
+        lfs_auth(nil, project)
+
+        expect(response).to have_http_status(404)
+      end
     end
 
     context 'deploy key' do
       let(:key) { create(:deploy_key) }
 
       it 'returns the correct information about the key' do
-        lfs_auth(key, project)
+        lfs_auth(key.id, project)
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
@@ -421,10 +427,10 @@ describe API::API, api: true  do
     )
   end
 
-  def lfs_auth(key, project)
+  def lfs_auth(key_id, project)
     post(
       api("/internal/lfs_authenticate"),
-      key_id: key.id,
+      key_id: key_id,
       secret_token: secret_token,
       project: project.path_with_namespace
     )

From e941365f3be88cebd57e9b08ba8702c1b688cb94 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 09:59:10 +0200
Subject: [PATCH 26/41] Rename capabilities to authentication_abilities

---
 app/controllers/jwt_controller.rb             |  2 +-
 .../projects/git_http_client_controller.rb    | 14 ++++++-----
 .../projects/git_http_controller.rb           |  2 +-
 ...ntainer_registry_authentication_service.rb | 12 +++++-----
 lib/api/internal.rb                           |  6 ++---
 lib/gitlab/auth.rb                            | 24 +++++++++----------
 lib/gitlab/git_access.rb                      | 12 +++++-----
 spec/lib/gitlab/auth_spec.rb                  | 22 ++++++++---------
 spec/lib/gitlab/git_access_spec.rb            | 20 ++++++++--------
 spec/lib/gitlab/git_access_wiki_spec.rb       |  4 ++--
 ...er_registry_authentication_service_spec.rb | 18 ++++++++++----
 11 files changed, 74 insertions(+), 62 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index a69534c2258..06d96774754 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -14,7 +14,7 @@ class JwtController < ApplicationController
     @authentication_result ||= Gitlab::Auth::Result.new
 
     result = service.new(@authentication_result.project, @authentication_result.actor, auth_params).
-      execute(capabilities: @authentication_result.capabilities)
+      execute(authentication_abilities: @authentication_result.authentication_abilities)
 
     render json: result, status: result[:http_status]
   end
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index d92d28b7e02..3cc915ecc2a 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :actor, :capabilities
+  attr_reader :actor, :authentication_abilities
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -125,7 +125,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     when :oauth
       if download_request?
         @actor = auth_result.actor
-        @capabilities = auth_result.capabilities
+        @authentication_abilities = auth_result.authentication_abilities
       else
         return false
       end
@@ -133,11 +133,13 @@ class Projects::GitHttpClientController < Projects::ApplicationController
       if download_request?
         @lfs_deploy_key = true
         @actor = auth_result.actor
-        @capabilities = auth_result.capabilities
+        @authentication_abilities = auth_result.authentication_abilities
+      else
+        return false
       end
     when :lfs_token, :personal_token, :gitlab_or_ldap, :build
       @actor = auth_result.actor
-      @capabilities = auth_result.capabilities
+      @authentication_abilities = auth_result.authentication_abilities
     else
       # Not allowed
       return false
@@ -150,8 +152,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @lfs_deploy_key && actor && actor.projects.include?(project)
   end
 
-  def has_capability?(capability)
-    @capabilities.include?(capability)
+  def has_authentication_ability?(capability)
+    @authentication_abilities.include?(capability)
   end
 
   def verify_workhorse_api!
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 89afaaed510..662d38b10a5 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -86,7 +86,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access
-    @access ||= Gitlab::GitAccess.new(user, project, 'http', capabilities: capabilities)
+    @access ||= Gitlab::GitAccess.new(user, project, 'http', authentication_abilities: authentication_abilities)
   end
 
   def access_check
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index df1c9b2851c..36120a5bc99 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -4,8 +4,8 @@ module Auth
 
     AUDIENCE = 'container_registry'
 
-    def execute(capabilities:)
-      @capabilities = capabilities || []
+    def execute(authentication_abilities:)
+      @authentication_abilities = authentication_abilities || []
 
       return error('not found', 404) unless registry.enabled
 
@@ -92,23 +92,23 @@ module Auth
       # Build can:
       # 1. pull from it's own project (for ex. a build)
       # 2. read images from dependent projects if creator of build is a team member
-      @capabilities.include?(:build_read_container_image) &&
+      @authentication_abilities.include?(:build_read_container_image) &&
         (requested_project == project || can?(current_user, :build_read_container_image, requested_project))
     end
 
     def user_can_pull?(requested_project)
-      @capabilities.include?(:read_container_image) &&
+      @authentication_abilities.include?(:read_container_image) &&
         can?(current_user, :read_container_image, requested_project)
     end
 
     def build_can_push?(requested_project)
       # Build can push only to project to from which he originates
-      @capabilities.include?(:build_create_container_image) &&
+      @authentication_abilities.include?(:build_create_container_image) &&
         requested_project == project
     end
 
     def user_can_push?(requested_project)
-      @capabilities.include?(:create_container_image) &&
+      @authentication_abilities.include?(:create_container_image) &&
         can?(current_user, :create_container_image, requested_project)
     end
   end
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 865379c51c4..090d04544da 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -36,7 +36,7 @@ module API
           end
         end
 
-        def ssh_capabilities
+        def ssh_authentication_abilities
           [
             :read_project,
             :download_code,
@@ -59,9 +59,9 @@ module API
 
         access =
           if wiki?
-            Gitlab::GitAccessWiki.new(actor, project, protocol, capabilities: ssh_capabilities)
+            Gitlab::GitAccessWiki.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities)
           else
-            Gitlab::GitAccess.new(actor, project, protocol, capabilities: ssh_capabilities)
+            Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities)
           end
 
         access_status = access.check(params[:action], params[:changes])
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index b14c4e565d5..3d7cc176e07 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:actor, :project, :type, :capabilities) do
+    Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
       def success?
         actor.present? || type == :ci
       end
@@ -77,7 +77,7 @@ module Gitlab
           service = project.public_send("#{underscored_service}_service")
 
           if service && service.activated? && service.valid_token?(password)
-            Result.new(nil, project, :ci, build_capabilities)
+            Result.new(nil, project, :ci, build_authentication_abilities)
           end
         end
       end
@@ -88,7 +88,7 @@ module Gitlab
 
         raise Gitlab::Auth::MissingPersonalTokenError if user.two_factor_enabled?
 
-        Result.new(user, nil, :gitlab_or_ldap, full_capabilities)
+        Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
       end
 
       def oauth_access_token_check(login, password)
@@ -96,7 +96,7 @@ module Gitlab
           token = Doorkeeper::AccessToken.by_token(password)
           if token && token.accessible?
             user = User.find_by(id: token.resource_owner_id)
-            Result.new(user, nil, :oauth, read_capabilities)
+            Result.new(user, nil, :oauth, read_authentication_abilities)
           end
         end
       end
@@ -105,7 +105,7 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, nil, :personal_token, full_capabilities) if user.present? && user == validation
+          Result.new(user, nil, :personal_token, full_authentication_abilities) if user.present? && user == validation
         end
       end
 
@@ -122,7 +122,7 @@ module Gitlab
         if actor
           token_handler = Gitlab::LfsToken.new(actor)
 
-          Result.new(actor, nil, token_handler.type, read_capabilities) if Devise.secure_compare(token_handler.value, password)
+          Result.new(actor, nil, token_handler.type, read_authentication_abilities) if Devise.secure_compare(token_handler.value, password)
         end
       end
 
@@ -136,14 +136,14 @@ module Gitlab
 
         if build.user
           # If user is assigned to build, use restricted credentials of user
-          Result.new(build.user, build.project, :build, build_capabilities)
+          Result.new(build.user, build.project, :build, build_authentication_abilities)
         else
           # Otherwise use generic CI credentials (backward compatibility)
-          Result.new(nil, build.project, :ci, build_capabilities)
+          Result.new(nil, build.project, :ci, build_authentication_abilities)
         end
       end
 
-      def build_capabilities
+      def build_authentication_abilities
         [
           :read_project,
           :build_download_code,
@@ -152,7 +152,7 @@ module Gitlab
         ]
       end
 
-      def read_capabilities
+      def read_authentication_abilities
         [
           :read_project,
           :download_code,
@@ -160,8 +160,8 @@ module Gitlab
         ]
       end
 
-      def full_capabilities
-        read_capabilities + [
+      def full_authentication_abilities
+        read_authentication_abilities + [
           :push_code,
           :update_container_image
         ]
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 21286e77dc6..799794c0171 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -5,13 +5,13 @@ module Gitlab
     DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
     PUSH_COMMANDS = %w{ git-receive-pack }
 
-    attr_reader :actor, :project, :protocol, :user_access, :capabilities
+    attr_reader :actor, :project, :protocol, :user_access, :authentication_abilities
 
-    def initialize(actor, project, protocol, capabilities:)
+    def initialize(actor, project, protocol, authentication_abilities:)
       @actor    = actor
       @project  = project
       @protocol = protocol
-      @capabilities = capabilities
+      @authentication_abilities = authentication_abilities
       @user_access = UserAccess.new(user, project: project)
     end
 
@@ -69,15 +69,15 @@ module Gitlab
     end
 
     def user_can_download_code?
-      capabilities.include?(:download_code) && user_access.can_do_action?(:download_code)
+      authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_code)
     end
 
     def build_can_download_code?
-      capabilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
+      authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
     def user_push_access_check(changes)
-      unless capabilities.include?(:push_code)
+      unless authentication_abilities.include?(:push_code)
         return build_status_object(false, "You are not allowed to upload code for this project.")
       end
 
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index e24ad530904..744282b2afa 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -16,13 +16,13 @@ describe Gitlab::Auth, lib: true do
         end
 
         it 'recognises user-less build' do
-          expect(subject).to eq(Gitlab::Auth::Result.new(nil, build.project, :ci, build_capabilities))
+          expect(subject).to eq(Gitlab::Auth::Result.new(nil, build.project, :ci, build_authentication_abilities))
         end
 
         it 'recognises user token' do
           build.update(user: create(:user))
 
-          expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, build_capabilities))
+          expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities))
         end
       end
 
@@ -48,7 +48,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'drone-ci-token')
-      expect(gl_auth.find_for_git_client('drone-ci-token', 'token', project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, project, :ci, build_capabilities))
+      expect(gl_auth.find_for_git_client('drone-ci-token', 'token', project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, project, :ci, build_authentication_abilities))
     end
 
     it 'recognizes master passwords' do
@@ -56,7 +56,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_capabilities))
+      expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities))
     end
 
     it 'recognizes user lfs tokens' do
@@ -65,7 +65,7 @@ describe Gitlab::Auth, lib: true do
       token = Gitlab::LfsToken.new(user).generate
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, read_capabilities))
+      expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, read_authentication_abilities))
     end
 
     it 'recognizes deploy key lfs tokens' do
@@ -74,7 +74,7 @@ describe Gitlab::Auth, lib: true do
       token = Gitlab::LfsToken.new(key).generate
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}")
-      expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_capabilities))
+      expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities))
     end
 
     it 'recognizes OAuth tokens' do
@@ -84,7 +84,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2')
-      expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_capabilities))
+      expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities))
     end
 
     it 'returns double nil for invalid credentials' do
@@ -149,7 +149,7 @@ describe Gitlab::Auth, lib: true do
 
   private
 
-  def build_capabilities
+  def build_authentication_abilities
     [
       :read_project,
       :build_download_code,
@@ -158,7 +158,7 @@ describe Gitlab::Auth, lib: true do
     ]
   end
 
-  def read_capabilities
+  def read_authentication_abilities
     [
       :read_project,
       :download_code,
@@ -166,8 +166,8 @@ describe Gitlab::Auth, lib: true do
     ]
   end
 
-  def full_capabilities
-    read_capabilities + [
+  def full_authentication_abilities
+    read_authentication_abilities + [
       :push_code,
       :update_container_image
     ]
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index c6fe56aac1c..ed43646330f 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -1,11 +1,11 @@
 require 'spec_helper'
 
 describe Gitlab::GitAccess, lib: true do
-  let(:access) { Gitlab::GitAccess.new(actor, project, 'web', capabilities: capabilities) }
+  let(:access) { Gitlab::GitAccess.new(actor, project, 'web', authentication_abilities: authentication_abilities) }
   let(:project) { create(:project) }
   let(:user) { create(:user) }
   let(:actor) { user }
-  let(:capabilities) do
+  let(:authentication_abilities) do
     [
       :read_project,
       :download_code,
@@ -22,7 +22,7 @@ describe Gitlab::GitAccess, lib: true do
     context 'ssh disabled' do
       before do
         disable_protocol('ssh')
-        @acc = Gitlab::GitAccess.new(actor, project, 'ssh', capabilities: capabilities)
+        @acc = Gitlab::GitAccess.new(actor, project, 'ssh', authentication_abilities: authentication_abilities)
       end
 
       it 'blocks ssh git push' do
@@ -37,7 +37,7 @@ describe Gitlab::GitAccess, lib: true do
     context 'http disabled' do
       before do
         disable_protocol('http')
-        @acc = Gitlab::GitAccess.new(actor, project, 'http', capabilities: capabilities)
+        @acc = Gitlab::GitAccess.new(actor, project, 'http', authentication_abilities: authentication_abilities)
       end
 
       it 'blocks http push' do
@@ -119,8 +119,8 @@ describe Gitlab::GitAccess, lib: true do
       end
     end
 
-    describe 'build capabilities permissions' do
-      let(:capabilities) { build_capabilities }
+    describe 'build authentication_abilities permissions' do
+      let(:authentication_abilities) { build_authentication_abilities }
 
       describe 'reporter user' do
         before { project.team << [user, :reporter] }
@@ -350,8 +350,8 @@ describe Gitlab::GitAccess, lib: true do
     end
   end
 
-  describe 'build capabilities permissions' do
-    let(:capabilities) { build_capabilities }
+  describe 'build authentication abilities' do
+    let(:authentication_abilities) { build_authentication_abilities }
 
     it_behaves_like 'can not push code' do
       def authorize
@@ -373,14 +373,14 @@ describe Gitlab::GitAccess, lib: true do
 
   private
 
-  def build_capabilities
+  def build_authentication_abilities
     [
       :read_project,
       :build_download_code
     ]
   end
 
-  def full_capabilities
+  def full_authentication_abilities
     [
       :read_project,
       :download_code,
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index 860e701c1a1..d05f0beb080 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -1,10 +1,10 @@
 require 'spec_helper'
 
 describe Gitlab::GitAccessWiki, lib: true do
-  let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', capabilities: capabilities) }
+  let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', authentication_abilities) }
   let(:project) { create(:project) }
   let(:user) { create(:user) }
-  let(:capabilities) do
+  let(:authentication_abilities) do
     [
       :read_project,
       :download_code,
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 2d39bd61b8f..c64df4979b0 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -6,14 +6,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
   let(:current_params) { {} }
   let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) }
   let(:payload) { JWT.decode(subject[:token], rsa_key).first }
-  let(:capabilities) do
+  let(:authentication_abilities) do
     [
       :read_container_image,
       :create_container_image
     ]
   end
 
-  subject { described_class.new(current_project, current_user, current_params).execute(capabilities: capabilities) }
+  subject { described_class.new(current_project, current_user, current_params).execute(authentication_abilities: authentication_abilities) }
 
   before do
     allow(Gitlab.config.registry).to receive_messages(enabled: true, issuer: 'rspec', key: nil)
@@ -198,7 +198,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
   context 'build authorized as user' do
     let(:current_project) { create(:empty_project) }
     let(:current_user) { create(:user) }
-    let(:capabilities) do
+    let(:authentication_abilities) do
       [
         :build_read_container_image,
         :build_create_container_image
@@ -255,7 +255,17 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
           context 'when you are admin' do
             let(:current_user) { create(:admin) }
 
-            it_behaves_like 'pullable for being team member'
+            context 'when you are not member' do
+              it_behaves_like 'an inaccessible'
+            end
+
+            context 'when you are member' do
+              before do
+                project.team << [current_user, :developer]
+              end
+
+              it_behaves_like 'a pullable'
+            end
           end
         end
       end

From a387ff7ba85dc75608ae5347aa405ea30b4e8c8c Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 11:06:31 +0200
Subject: [PATCH 27/41] Fix specs after renaming authentication_capabilities

---
 app/helpers/lfs_helper.rb               |  6 +++---
 spec/lib/gitlab/auth_spec.rb            | 18 ++++++++++--------
 spec/lib/gitlab/git_access_wiki_spec.rb |  2 +-
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 0d6b72ff746..018ca7d7bba 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -29,17 +29,17 @@ module LfsHelper
   end
 
   def user_can_download_code?
-    has_capability?(:download_code) && user && user.can?(:download_code, project)
+    has_authentication_ability?(:download_code) && can?(user, :download_code, project)
   end
 
   def build_can_download_code?
-    has_capability?(:build_download_code) && user && user.can?(:build_download_code, project)
+    has_authentication_ability?(:build_download_code) && can?(user, :build_download_code, project)
   end
 
   def lfs_upload_access?
     return false unless project.lfs_enabled?
 
-    has_capability?(:push_code) && user && user.can?(:push_code, project)
+    has_authentication_ability?(:push_code) && can?(user, :push_code, project)
   end
 
   def render_lfs_forbidden
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 744282b2afa..d3707005a0e 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -26,16 +26,18 @@ describe Gitlab::Auth, lib: true do
         end
       end
 
-      context 'for non-running build' do
-        let!(:build) { create(:ci_build, :pending) }
-        let(:project) { build.project }
+      (HasStatus::AVAILABLE_STATUSES - [:running]).each do |build_status|
+        context "for #{build_status} build" do
+          let!(:build) { create(:ci_build, status: build_status) }
+          let(:project) { build.project }
 
-        before do
-          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'gitlab-ci-token')
-        end
+          before do
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'gitlab-ci-token')
+          end
 
-        it 'denies authentication' do
-          expect(subject).to eq(Gitlab::Auth::Result.new)
+          it 'denies authentication' do
+            expect(subject).not_to eq(Gitlab::Auth::Result.new)
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index d05f0beb080..576cda595bb 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::GitAccessWiki, lib: true do
-  let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', authentication_abilities) }
+  let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', authentication_abilities: authentication_abilities) }
   let(:project) { create(:project) }
   let(:user) { create(:user) }
   let(:authentication_abilities) do

From 1954cb80fd168b7d7cd3a446782fdaf30d0b3f08 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 11:06:57 +0200
Subject: [PATCH 28/41] Added missing LFS specs

---
 spec/requests/lfs_http_spec.rb | 235 ++++++++++++++++++++++++++++++---
 1 file changed, 213 insertions(+), 22 deletions(-)

diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 8ead97efb01..cad8b914668 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -15,7 +15,6 @@ describe 'Git LFS API and storage' do
   let(:authorization) { }
   let(:sendfile) { }
   let(:pipeline) { create(:ci_empty_pipeline, project: project) }
-  let(:build) { create(:ci_build, :running, pipeline: pipeline) }
 
   let(:sample_oid) { lfs_object.oid }
   let(:sample_size) { lfs_object.size }
@@ -258,14 +257,63 @@ describe 'Git LFS API and storage' do
           it_behaves_like 'responds with a file'
         end
 
-        context 'when build is authorized' do
+        context 'when build is authorized as' do
           let(:authorization) { authorize_ci_project }
 
-          let(:update_permissions) do
-            project.lfs_objects << lfs_object
+          shared_examples 'can download LFS only from own projects' do
+            context 'for own project' do
+              let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+
+              let(:update_permissions) do
+                project.team << [user, :reporter]
+                project.lfs_objects << lfs_object
+              end
+
+              it_behaves_like 'responds with a file'
+            end
+
+            context 'for other project' do
+              let(:other_project) { create(:empty_project) }
+              let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
+
+              let(:update_permissions) do
+                project.lfs_objects << lfs_object
+              end
+
+              it 'rejects downloading code' do
+                expect(response).to have_http_status(other_project_status)
+              end
+            end
           end
 
-          it_behaves_like 'responds with a file'
+          context 'administrator' do
+            let(:user) { create(:admin) }
+            let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+            it_behaves_like 'can download LFS only from own projects' do
+              # We render 403, because administrator does have normally access
+              let(:other_project_status) { 403 }
+            end
+          end
+
+          context 'regular user' do
+            let(:user) { create(:user) }
+            let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+            it_behaves_like 'can download LFS only from own projects' do
+              # We render 404, to prevent data leakage about existence of the project
+              let(:other_project_status) { 404 }
+            end
+          end
+
+          context 'does not have user' do
+            let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+            it_behaves_like 'can download LFS only from own projects' do
+              # We render 401, to prevent data leakage about existence of the project
+              let(:other_project_status) { 401 }
+            end
+          end
         end
       end
 
@@ -445,10 +493,62 @@ describe 'Git LFS API and storage' do
         end
       end
 
-      context 'when CI is authorized' do
+      context 'when build is authorized as' do
         let(:authorization) { authorize_ci_project }
 
-        it_behaves_like 'an authorized requests'
+        let(:update_lfs_permissions) do
+          project.lfs_objects << lfs_object
+        end
+
+        shared_examples 'can download LFS only from own projects' do
+          context 'for own project' do
+            let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+
+            let(:update_user_permissions) do
+              project.team << [user, :reporter]
+            end
+
+            it_behaves_like 'an authorized requests'
+          end
+
+          context 'for other project' do
+            let(:other_project) { create(:empty_project) }
+            let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
+
+            it 'rejects downloading code' do
+              expect(response).to have_http_status(other_project_status)
+            end
+          end
+        end
+
+        context 'administrator' do
+          let(:user) { create(:admin) }
+          let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+          it_behaves_like 'can download LFS only from own projects' do
+            # We render 403, because administrator does have normally access
+            let(:other_project_status) { 403 }
+          end
+        end
+
+        context 'regular user' do
+          let(:user) { create(:user) }
+          let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+          it_behaves_like 'can download LFS only from own projects' do
+            # We render 404, to prevent data leakage about existence of the project
+            let(:other_project_status) { 404 }
+          end
+        end
+
+        context 'does not have user' do
+          let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+          it_behaves_like 'can download LFS only from own projects' do
+            # We render 401, to prevent data leakage about existence of the project
+            let(:other_project_status) { 401 }
+          end
+        end
       end
 
       context 'when user is not authenticated' do
@@ -597,11 +697,37 @@ describe 'Git LFS API and storage' do
           end
         end
 
-        context 'when CI is authorized' do
+        context 'when build is authorized' do
           let(:authorization) { authorize_ci_project }
 
-          it 'responds with 401' do
-            expect(response).to have_http_status(401)
+          context 'build has an user' do
+            let(:user) { create(:user) }
+
+            context 'tries to push to own project' do
+              let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+              it 'responds with 403' do
+                expect(response).to have_http_status(403)
+              end
+            end
+
+            context 'tries to push to other project' do
+              let(:other_project) { create(:empty_project) }
+              let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
+              let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+              it 'responds with 403' do
+                expect(response).to have_http_status(403)
+              end
+            end
+          end
+
+          context 'does not have user' do
+            let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+            it 'responds with 401' do
+              expect(response).to have_http_status(401)
+            end
           end
         end
       end
@@ -623,14 +749,6 @@ describe 'Git LFS API and storage' do
           end
         end
       end
-
-      context 'when CI is authorized' do
-        let(:authorization) { authorize_ci_project }
-
-        it 'responds with status 401' do
-          expect(response).to have_http_status(401)
-        end
-      end
     end
 
     describe 'unsupported' do
@@ -793,10 +911,51 @@ describe 'Git LFS API and storage' do
         end
       end
 
-      context 'when CI is authenticated' do
+      context 'when build is authorized' do
         let(:authorization) { authorize_ci_project }
 
-        it_behaves_like 'unauthorized'
+        context 'build has an user' do
+          let(:user) { create(:user) }
+
+          context 'tries to push to own project' do
+            let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+            before do
+              project.team << [user, :developer]
+              put_authorize
+            end
+
+            it 'responds with 403' do
+              expect(response).to have_http_status(403)
+            end
+          end
+
+          context 'tries to push to other project' do
+            let(:other_project) { create(:empty_project) }
+            let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
+            let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+            before do
+              put_authorize
+            end
+
+            it 'responds with 404' do
+              expect(response).to have_http_status(404)
+            end
+          end
+        end
+
+        context 'does not have user' do
+          let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+          before do
+            put_authorize
+          end
+
+          it 'responds with 401' do
+            expect(response).to have_http_status(401)
+          end
+        end
       end
 
       context 'for unauthenticated' do
@@ -853,10 +1012,42 @@ describe 'Git LFS API and storage' do
         end
       end
 
-      context 'when CI is authenticated' do
+      context 'when build is authorized' do
         let(:authorization) { authorize_ci_project }
 
-        it_behaves_like 'unauthorized'
+        before do
+          put_authorize
+        end
+
+        context 'build has an user' do
+          let(:user) { create(:user) }
+
+          context 'tries to push to own project' do
+            let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+            it 'responds with 403' do
+              expect(response).to have_http_status(403)
+            end
+          end
+
+          context 'tries to push to other project' do
+            let(:other_project) { create(:empty_project) }
+            let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
+            let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+
+            it 'responds with 403' do
+              expect(response).to have_http_status(403)
+            end
+          end
+        end
+
+        context 'does not have user' do
+          let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+          it 'responds with 401' do
+            expect(response).to have_http_status(401)
+          end
+        end
       end
 
       context 'for unauthenticated' do

From 9d8afa222c678a2222f5219458759897089d7dad Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 12:46:33 +0200
Subject: [PATCH 29/41] Improve code comments

---
 app/models/ci/build.rb                                        | 2 +-
 app/policies/project_policy.rb                                | 2 +-
 .../auth/container_registry_authentication_service.rb         | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 0b017c98916..57ef4646d24 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -179,7 +179,7 @@ module Ci
     end
 
     def repo_url
-      auth = "gitlab-ci-token:#{ensure_token}@"
+      auth = "gitlab-ci-token:#{ensure_token!}@"
       project.http_url_to_repo.sub(/^https?:\/\//) do |prefix|
         prefix + auth
       end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index ce686af2ade..00c4c7b1440 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -64,7 +64,7 @@ class ProjectPolicy < BasePolicy
     can! :read_deployment
   end
 
-  # Permissions given when an user is direct member of a group
+  # Permissions given when an user is team member of a project
   def team_member_reporter_access!
     can! :build_download_code
     can! :build_read_container_image
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 36120a5bc99..98da6563947 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -90,7 +90,7 @@ module Auth
 
     def build_can_pull?(requested_project)
       # Build can:
-      # 1. pull from it's own project (for ex. a build)
+      # 1. pull from its own project (for ex. a build)
       # 2. read images from dependent projects if creator of build is a team member
       @authentication_abilities.include?(:build_read_container_image) &&
         (requested_project == project || can?(current_user, :build_read_container_image, requested_project))
@@ -102,7 +102,7 @@ module Auth
     end
 
     def build_can_push?(requested_project)
-      # Build can push only to project to from which he originates
+      # Build can push only to the project from which it originates
       @authentication_abilities.include?(:build_create_container_image) &&
         requested_project == project
     end

From f7ae37c1d092f89cd9b9dc24be95670abed16ffc Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 13:34:05 +0200
Subject: [PATCH 30/41] Simplify checking of allowed abilities in
 git_http_client_controller

---
 .../projects/git_http_client_controller.rb    | 75 ++++++++-----------
 lib/gitlab/auth.rb                            | 10 +++
 spec/requests/git_http_spec.rb                |  2 +-
 spec/requests/lfs_http_spec.rb                | 32 ++++----
 4 files changed, 60 insertions(+), 59 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 3cc915ecc2a..632dac6aac9 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,11 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :actor, :authentication_abilities
+  attr_reader :authentication_result
+
+  delegate :actor, :authentication_abilities, to: :authentication_result, allow_nil: true
+
+  alias_method :user, :actor
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token
@@ -26,9 +30,12 @@ class Projects::GitHttpClientController < Projects::ApplicationController
         return # Allow access
       end
     elsif allow_kerberos_spnego_auth? && spnego_provided?
-      @actor = find_kerberos_user
+      user = find_kerberos_user
+
+      if user
+        @authentication_result = Gitlab::Auth::Result.new(
+          user, nil, :kerberos, Gitlab::Auth.full_authentication_abilities)
 
-      if actor
         send_final_spnego_response
         return # Allow access
       end
@@ -104,56 +111,40 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     render plain: 'Not Found', status: :not_found
   end
 
-  def ci?
-    @ci
-  end
-
-  def user
-    @actor
-  end
-
   def handle_basic_authentication(login, password)
-    auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
+    @authentication_result = Gitlab::Auth.find_for_git_client(
+      login, password, project: project, ip: request.ip)
 
-    case auth_result.type
-    when :ci
-      if auth_result.project == project && download_request?
-        @ci = true
-      else
-        return false
-      end
-    when :oauth
-      if download_request?
-        @actor = auth_result.actor
-        @authentication_abilities = auth_result.authentication_abilities
-      else
-        return false
-      end
-    when :lfs_deploy_token
-      if download_request?
-        @lfs_deploy_key = true
-        @actor = auth_result.actor
-        @authentication_abilities = auth_result.authentication_abilities
-      else
-        return false
-      end
-    when :lfs_token, :personal_token, :gitlab_or_ldap, :build
-      @actor = auth_result.actor
-      @authentication_abilities = auth_result.authentication_abilities
+    return false unless @authentication_result.success?
+
+    if download_request?
+      authentication_has_download_access?
     else
-      # Not allowed
-      return false
+      authentication_has_upload_access?
     end
+  end
 
-    true
+  def authentication_has_download_access?
+    has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code)
+  end
+
+  def authentication_has_upload_access?
+    has_authentication_ability?(:push_code)
+  end
+
+  def ci?
+    authentication_result && authentication_result.ci? &&
+      authentication_result.project && authentication_result.project == project
   end
 
   def lfs_deploy_key?
-    @lfs_deploy_key && actor && actor.projects.include?(project)
+    authentication_result && authentication_result.lfs_deploy_token? &&
+      actor && actor.projects.include?(project)
   end
 
   def has_authentication_ability?(capability)
-    @authentication_abilities.include?(capability)
+    authentication_abilities &&
+      authentication_abilities.include?(capability)
   end
 
   def verify_workhorse_api!
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 3d7cc176e07..f9ae5e4543f 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,14 @@
 module Gitlab
   module Auth
     Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
+      def ci?
+        type == :ci
+      end
+
+      def lfs_deploy_token?
+        type == :lfs_deploy_token
+      end
+
       def success?
         actor.present? || type == :ci
       end
@@ -143,6 +151,8 @@ module Gitlab
         end
       end
 
+      public
+
       def build_authentication_abilities
         [
           :read_project,
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 0311755dd06..f828e898740 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -346,7 +346,7 @@ describe 'Git HTTP requests', lib: true do
             it 'uploads get status 403' do
               push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
-              expect(response).to have_http_status(403)
+              expect(response).to have_http_status(401)
             end
           end
 
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index cad8b914668..09e4e265dd1 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -310,8 +310,8 @@ describe 'Git LFS API and storage' do
             let(:build) { create(:ci_build, :running, pipeline: pipeline) }
 
             it_behaves_like 'can download LFS only from own projects' do
-              # We render 401, to prevent data leakage about existence of the project
-              let(:other_project_status) { 401 }
+              # We render 404, to prevent data leakage about existence of the project
+              let(:other_project_status) { 404 }
             end
           end
         end
@@ -545,8 +545,8 @@ describe 'Git LFS API and storage' do
           let(:build) { create(:ci_build, :running, pipeline: pipeline) }
 
           it_behaves_like 'can download LFS only from own projects' do
-            # We render 401, to prevent data leakage about existence of the project
-            let(:other_project_status) { 401 }
+            # We render 404, to prevent data leakage about existence of the project
+            let(:other_project_status) { 404 }
           end
         end
       end
@@ -706,8 +706,8 @@ describe 'Git LFS API and storage' do
             context 'tries to push to own project' do
               let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
 
-              it 'responds with 403' do
-                expect(response).to have_http_status(403)
+              it 'responds with 401' do
+                expect(response).to have_http_status(401)
               end
             end
 
@@ -716,8 +716,8 @@ describe 'Git LFS API and storage' do
               let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
               let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
 
-              it 'responds with 403' do
-                expect(response).to have_http_status(403)
+              it 'responds with 401' do
+                expect(response).to have_http_status(401)
               end
             end
           end
@@ -925,8 +925,8 @@ describe 'Git LFS API and storage' do
               put_authorize
             end
 
-            it 'responds with 403' do
-              expect(response).to have_http_status(403)
+            it 'responds with 401' do
+              expect(response).to have_http_status(401)
             end
           end
 
@@ -939,8 +939,8 @@ describe 'Git LFS API and storage' do
               put_authorize
             end
 
-            it 'responds with 404' do
-              expect(response).to have_http_status(404)
+            it 'responds with 401' do
+              expect(response).to have_http_status(401)
             end
           end
         end
@@ -1025,8 +1025,8 @@ describe 'Git LFS API and storage' do
           context 'tries to push to own project' do
             let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
 
-            it 'responds with 403' do
-              expect(response).to have_http_status(403)
+            it 'responds with 401' do
+              expect(response).to have_http_status(401)
             end
           end
 
@@ -1035,8 +1035,8 @@ describe 'Git LFS API and storage' do
             let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
             let(:build) { create(:ci_build, :running, pipeline: pipeline, user: user) }
 
-            it 'responds with 403' do
-              expect(response).to have_http_status(403)
+            it 'responds with 401' do
+              expect(response).to have_http_status(401)
             end
           end
         end

From b0195d5c55d913dd62cb01b553b045f2681e7eb7 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 13:47:54 +0200
Subject: [PATCH 31/41] Fix specs for available statuses

---
 spec/lib/gitlab/auth_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index d3707005a0e..1af5abe9f15 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Auth, lib: true do
         end
       end
 
-      (HasStatus::AVAILABLE_STATUSES - [:running]).each do |build_status|
+      (HasStatus::AVAILABLE_STATUSES - ['running']).each do |build_status|
         context "for #{build_status} build" do
           let!(:build) { create(:ci_build, status: build_status) }
           let(:project) { build.project }
@@ -36,7 +36,7 @@ describe Gitlab::Auth, lib: true do
           end
 
           it 'denies authentication' do
-            expect(subject).not_to eq(Gitlab::Auth::Result.new)
+            expect(subject).to eq(Gitlab::Auth::Result.new)
           end
         end
       end

From 2742f9fb98babc0009e446d291757ae43c54c101 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 16 Sep 2016 16:07:21 +0200
Subject: [PATCH 32/41] Improve authentication_result usage

---
 .../projects/git_http_client_controller.rb    | 31 ++++++++++++-------
 spec/requests/git_http_spec.rb                |  4 +--
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 632dac6aac9..ee9ea4bc8b2 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -19,6 +19,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   private
 
   def authenticate_user
+    @authentication_result = Gitlab::Auth::Result.new
+
     if project && project.public? && download_request?
       return # Allow access
     end
@@ -124,6 +126,18 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     end
   end
 
+  def ci?
+    authentication_result.ci? &&
+      authentication_project &&
+      authentication_project == project
+  end
+
+  def lfs_deploy_key?
+    authentication_result.lfs_deploy_token? &&
+      actor &&
+      actor.projects.include?(project)
+  end
+
   def authentication_has_download_access?
     has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code)
   end
@@ -132,19 +146,12 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     has_authentication_ability?(:push_code)
   end
 
-  def ci?
-    authentication_result && authentication_result.ci? &&
-      authentication_result.project && authentication_result.project == project
-  end
-
-  def lfs_deploy_key?
-    authentication_result && authentication_result.lfs_deploy_token? &&
-      actor && actor.projects.include?(project)
-  end
-
   def has_authentication_ability?(capability)
-    authentication_abilities &&
-      authentication_abilities.include?(capability)
+    (authentication_abilities || []).include?(capability)
+  end
+
+  def authentication_project
+    authentication_result.project
   end
 
   def verify_workhorse_api!
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index f828e898740..e3922bec689 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -322,10 +322,10 @@ describe 'Git HTTP requests', lib: true do
             expect(response).to have_http_status(401)
           end
 
-          it "downloads from other project get status 401" do
+          it "downloads from other project get status 404" do
             clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
-            expect(response).to have_http_status(401)
+            expect(response).to have_http_status(404)
           end
         end
 

From 0ca43b1b86edea69656582b2a8febb0d41f7ef01 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 12:37:46 +0200
Subject: [PATCH 33/41] Fix permissions for creating container images

---
 lib/gitlab/auth.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index f9ae5e4543f..150a4ead45d 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -173,7 +173,7 @@ module Gitlab
       def full_authentication_abilities
         read_authentication_abilities + [
           :push_code,
-          :update_container_image
+          :create_container_image
         ]
       end
     end

From b51ededc5fef05f94a632aa7651b5a1f7395bd4e Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 12:38:03 +0200
Subject: [PATCH 34/41] Don't leak build tokens in build logs

---
 app/controllers/projects/builds_controller.rb |  6 +-
 app/models/ci/build.rb                        | 16 ++--
 lib/ci/mask_secret.rb                         |  9 +++
 spec/lib/ci/mask_secret_spec.rb               | 19 +++++
 spec/models/build_spec.rb                     | 78 +++++++++++++++++--
 5 files changed, 113 insertions(+), 15 deletions(-)
 create mode 100644 lib/ci/mask_secret.rb
 create mode 100644 spec/lib/ci/mask_secret_spec.rb

diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index 77934ff9962..9ce5b4de42f 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -35,7 +35,11 @@ class Projects::BuildsController < Projects::ApplicationController
     respond_to do |format|
       format.html
       format.json do
-        render json: @build.to_json(methods: :trace_html)
+        render json: {
+          id: @build.id,
+          status: @build.status,
+          trace_html: @build.trace_html
+        }
       end
     end
   end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 57ef4646d24..8a9d7555393 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -241,12 +241,7 @@ module Ci
     end
 
     def trace
-      trace = raw_trace
-      if project && trace.present? && project.runners_token.present?
-        trace.gsub(project.runners_token, 'xxxxxx')
-      else
-        trace
-      end
+      hide_secrets(raw_trace)
     end
 
     def trace_length
@@ -259,6 +254,7 @@ module Ci
 
     def trace=(trace)
       recreate_trace_dir
+      trace = hide_secrets(trace)
       File.write(path_to_trace, trace)
     end
 
@@ -272,6 +268,8 @@ module Ci
     def append_trace(trace_part, offset)
       recreate_trace_dir
 
+      trace_part = hide_secrets(trace_part)
+
       File.truncate(path_to_trace, offset) if File.exist?(path_to_trace)
       File.open(path_to_trace, 'ab') do |f|
         f.write(trace_part)
@@ -490,5 +488,11 @@ module Ci
 
       pipeline.config_processor.build_attributes(name)
     end
+
+    def hide_secrets(trace)
+      trace = Ci::MaskSecret.mask(trace, project.runners_token) if project
+      trace = Ci::MaskSecret.mask(trace, token)
+      trace
+    end
   end
 end
diff --git a/lib/ci/mask_secret.rb b/lib/ci/mask_secret.rb
new file mode 100644
index 00000000000..3da04edde70
--- /dev/null
+++ b/lib/ci/mask_secret.rb
@@ -0,0 +1,9 @@
+module Ci::MaskSecret
+  class << self
+    def mask(value, token)
+      return value unless value.present? && token.present?
+
+      value.gsub(token, 'x' * token.length)
+    end
+  end
+end
diff --git a/spec/lib/ci/mask_secret_spec.rb b/spec/lib/ci/mask_secret_spec.rb
new file mode 100644
index 00000000000..518de76911c
--- /dev/null
+++ b/spec/lib/ci/mask_secret_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe Ci::MaskSecret, lib: true do
+  subject { described_class }
+
+  describe '#mask' do
+    it 'masks exact number of characters' do
+      expect(subject.mask('token', 'oke')).to eq('txxxn')
+    end
+
+    it 'masks multiple occurrences' do
+      expect(subject.mask('token token token', 'oke')).to eq('txxxn txxxn txxxn')
+    end
+
+    it 'does not mask if not found' do
+      expect(subject.mask('token', 'not')).to eq('token')
+    end
+  end
+end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 8eab4281bc7..e7864b7ad33 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -88,9 +88,7 @@ describe Ci::Build, models: true do
   end
 
   describe '#trace' do
-    subject { build.trace_html }
-
-    it { is_expected.to be_empty }
+    it { expect(build.trace).to be_nil }
 
     context 'when build.trace contains text' do
       let(:text) { 'example output' }
@@ -98,16 +96,80 @@ describe Ci::Build, models: true do
         build.trace = text
       end
 
-      it { is_expected.to include(text) }
-      it { expect(subject.length).to be >= text.length }
+      it { expect(build.trace).to eq(text) }
     end
 
-    context 'when build.trace hides token' do
+    context 'when build.trace hides runners token' do
       let(:token) { 'my_secret_token' }
 
       before do
-        build.project.update_attributes(runners_token: token)
-        build.update_attributes(trace: token)
+        build.update(trace: token)
+        build.project.update(runners_token: token)
+      end
+
+      it { expect(build.trace).not_to include(token) }
+      it { expect(build.raw_trace).to include(token) }
+    end
+
+    context 'when build.trace hides build token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.update(trace: token)
+        build.update(token: token)
+      end
+
+      it { expect(build.trace).not_to include(token) }
+      it { expect(build.raw_trace).to include(token) }
+    end
+  end
+
+  describe '#raw_trace' do
+    subject { build.raw_trace }
+
+    context 'when build.trace hides runners token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.project.update(runners_token: token)
+        build.update(trace: token)
+      end
+
+      it { is_expected.not_to include(token) }
+    end
+
+    context 'when build.trace hides build token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.update(token: token)
+        build.update(trace: token)
+      end
+
+      it { is_expected.not_to include(token) }
+    end
+  end
+
+  context '#append_trace' do
+    subject { build.trace_html }
+
+    context 'when build.trace hides runners token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.project.update(runners_token: token)
+        build.append_trace(token, 0)
+      end
+
+      it { is_expected.not_to include(token) }
+    end
+
+    context 'when build.trace hides build token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.update(token: token)
+        build.append_trace(token, 0)
       end
 
       it { is_expected.not_to include(token) }

From 5790684d1f81ca9fa63a10f3fec6339ef0092627 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 13:11:11 +0200
Subject: [PATCH 35/41] Support pushing via SSH

---
 lib/gitlab/auth.rb      | 9 ++++++++-
 lib/gitlab/lfs_token.rb | 4 ++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 150a4ead45d..1e0a7ec253a 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -130,7 +130,14 @@ module Gitlab
         if actor
           token_handler = Gitlab::LfsToken.new(actor)
 
-          Result.new(actor, nil, token_handler.type, read_authentication_abilities) if Devise.secure_compare(token_handler.value, password)
+          authentication_abilities =
+            if token_handler.user?
+              full_authentication_abilities
+            else
+              read_authentication_abilities
+            end
+
+          Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.value, password)
         end
       end
 
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index f492754b1c8..d089a2f9b0b 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -33,6 +33,10 @@ module Gitlab
       end
     end
 
+    def user?
+      actor.is_a?(User)
+    end
+
     def type
       actor.is_a?(User) ? :lfs_token : :lfs_deploy_token
     end

From 748dd35c65b0a7f3fbb0832fd18933ff8c19ef7d Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 13:29:48 +0200
Subject: [PATCH 36/41] Fix spec failures

---
 spec/lib/gitlab/auth_spec.rb | 2 +-
 spec/models/ci/build_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 1af5abe9f15..21f0d46100e 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -171,7 +171,7 @@ describe Gitlab::Auth, lib: true do
   def full_authentication_abilities
     read_authentication_abilities + [
       :push_code,
-      :update_container_image
+      :create_container_image
     ]
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index bce18b4e99e..a37a00f461a 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -8,7 +8,7 @@ describe Ci::Build, models: true do
     it 'obfuscates project runners token' do
       allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}")
 
-      expect(build.trace).to eq("Test: xxxxxx")
+      expect(build.trace).to eq("Test: xxxxxxxxxxxxxxxxxxxx")
     end
 
     it 'empty project runners token' do

From abcc0ba57027026cb5bb074b8691510a24724958 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 13:40:46 +0200
Subject: [PATCH 37/41] Added CHANGELOG

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 8eaeae0cb9c..cc54378f9f1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,7 @@ v 8.12.0 (unreleased)
   - Instructions for enabling Git packfile bitmaps !6104
   - Use Search::GlobalService.new in the `GET /projects/search/:query` endpoint
   - Fix pagination on user snippets page
+  - Run CI builds with the permissions of users !5735
   - Fix sorting of issues in API
   - Sort project variables by key. !6275 (Diego Souza)
   - Ensure specs on sorting of issues in API are deterministic on MySQL

From 79f60e2b5cf388416cdc5948e19ae0401f97d353 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 13:42:10 +0200
Subject: [PATCH 38/41] Move Gitlab::Auth.Result to separate file

---
 lib/gitlab/auth.rb        | 14 --------------
 lib/gitlab/auth/result.rb | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 14 deletions(-)
 create mode 100644 lib/gitlab/auth/result.rb

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 1e0a7ec253a..ca2a0920c00 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,19 +1,5 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
-      def ci?
-        type == :ci
-      end
-
-      def lfs_deploy_token?
-        type == :lfs_deploy_token
-      end
-
-      def success?
-        actor.present? || type == :ci
-      end
-    end
-
     class MissingPersonalTokenError < StandardError; end
 
     class << self
diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb
new file mode 100644
index 00000000000..e4786b12676
--- /dev/null
+++ b/lib/gitlab/auth/result.rb
@@ -0,0 +1,17 @@
+module Gitlab
+  module Auth
+    Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
+      def ci?
+        type == :ci
+      end
+
+      def lfs_deploy_token?
+        type == :lfs_deploy_token
+      end
+
+      def success?
+        actor.present? || type == :ci
+      end
+    end
+  end
+end

From 6d43c95b7011ec7ec4600e00bdc8df76bb39813c Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 13:38:58 +0200
Subject: [PATCH 39/41] Revert all changes introduced by
 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6043

---
 CHANGELOG                                     |  1 -
 .../projects/git_http_client_controller.rb    |  6 ---
 app/helpers/lfs_helper.rb                     |  2 +-
 doc/workflow/lfs/lfs_administration.md        |  4 +-
 .../lfs/manage_large_binaries_with_git_lfs.md |  8 ---
 lib/api/internal.rb                           | 13 -----
 lib/gitlab/auth.rb                            | 25 ---------
 lib/gitlab/auth/result.rb                     |  4 --
 lib/gitlab/lfs_token.rb                       | 54 -------------------
 spec/lib/gitlab/auth_spec.rb                  | 18 -------
 spec/lib/gitlab/lfs_token_spec.rb             | 51 ------------------
 spec/requests/api/internal_spec.rb            | 46 ----------------
 spec/requests/lfs_http_spec.rb                | 16 ------
 13 files changed, 3 insertions(+), 245 deletions(-)
 delete mode 100644 lib/gitlab/lfs_token.rb
 delete mode 100644 spec/lib/gitlab/lfs_token_spec.rb

diff --git a/CHANGELOG b/CHANGELOG
index cc54378f9f1..057c4bffda1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -106,7 +106,6 @@ v 8.12.0 (unreleased)
   - Remove green outline from `New branch unavailable` button on issue page !5858 (winniehell)
   - Fix repo title alignment (ClemMakesApps)
   - Change update interval of contacted_at
-  - Add LFS support to SSH !6043
   - Fix branch title trailing space on hover (ClemMakesApps)
   - Don't include 'Created By' tag line when importing from GitHub if there is a linked GitLab account (EspadaV8)
   - Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison)
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index ee9ea4bc8b2..d1a2c52d80a 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -132,12 +132,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
       authentication_project == project
   end
 
-  def lfs_deploy_key?
-    authentication_result.lfs_deploy_token? &&
-      actor &&
-      actor.projects.include?(project)
-  end
-
   def authentication_has_download_access?
     has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code)
   end
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 018ca7d7bba..8e827664681 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,7 +25,7 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    project.public? || ci? || lfs_deploy_key? || user_can_download_code? || build_can_download_code?
+    project.public? || ci? || user_can_download_code? || build_can_download_code?
   end
 
   def user_can_download_code?
diff --git a/doc/workflow/lfs/lfs_administration.md b/doc/workflow/lfs/lfs_administration.md
index b3c73e947f0..9dc1e9b47e3 100644
--- a/doc/workflow/lfs/lfs_administration.md
+++ b/doc/workflow/lfs/lfs_administration.md
@@ -45,5 +45,5 @@ In `config/gitlab.yml`:
 * Currently, storing GitLab Git LFS objects on a non-local storage (like S3 buckets)
   is not supported
 * Currently, removing LFS objects from GitLab Git LFS storage is not supported
-* LFS authentications via SSH was added with GitLab 8.12
-* Only compatible with the GitLFS client versions 1.1.0 and up, or 1.0.2.
+* LFS authentications via SSH is not supported for the time being
+* Only compatible with the GitLFS client versions 1.1.0 or 1.0.2.
diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
index 1a4f213a792..9fe065fa680 100644
--- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
+++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
@@ -35,10 +35,6 @@ Documentation for GitLab instance administrators is under [LFS administration do
   credentials store is recommended
 * Git LFS always assumes HTTPS so if you have GitLab server on HTTP you will have
   to add the URL to Git config manually (see #troubleshooting)
-  
->**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication
- still goes over HTTP, but now the SSH client passes the correct credentials
- to the Git LFS client, so no action is required by the user.
 
 ## Using Git LFS
 
@@ -136,10 +132,6 @@ git config --add lfs.url "http://gitlab.example.com/group/project.git/info/lfs"
 
 ### Credentials are always required when pushing an object
 
->**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication
- still goes over HTTP, but now the SSH client passes the correct credentials
- to the Git LFS client, so no action is required by the user.
-
 Given that Git LFS uses HTTP Basic Authentication to authenticate the user pushing
 the LFS object on every push for every object, user HTTPS credentials are required.
 
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 090d04544da..1114fd21784 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -82,19 +82,6 @@ module API
         response
       end
 
-      post "/lfs_authenticate" do
-        status 200
-
-        key = Key.find(params[:key_id])
-        token_handler = Gitlab::LfsToken.new(key)
-
-        {
-          username: token_handler.actor_name,
-          lfs_token: token_handler.generate,
-          repository_http_path: project.http_url_to_repo
-        }
-      end
-
       get "/merge_request_urls" do
         ::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
       end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index ca2a0920c00..7464d6082cb 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -11,7 +11,6 @@ module Gitlab
           build_access_token_check(login, password) ||
           user_with_password_for_git(login, password) ||
           oauth_access_token_check(login, password) ||
-          lfs_token_check(login, password) ||
           personal_access_token_check(login, password) ||
           Result.new
 
@@ -103,30 +102,6 @@ module Gitlab
         end
       end
 
-      def lfs_token_check(login, password)
-        deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/)
-
-        actor =
-          if deploy_key_matches
-            DeployKey.find(deploy_key_matches[1])
-          else
-            User.by_login(login)
-          end
-
-        if actor
-          token_handler = Gitlab::LfsToken.new(actor)
-
-          authentication_abilities =
-            if token_handler.user?
-              full_authentication_abilities
-            else
-              read_authentication_abilities
-            end
-
-          Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.value, password)
-        end
-      end
-
       def build_access_token_check(login, password)
         return unless login == 'gitlab-ci-token'
         return unless password
diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb
index e4786b12676..bf625649cbf 100644
--- a/lib/gitlab/auth/result.rb
+++ b/lib/gitlab/auth/result.rb
@@ -5,10 +5,6 @@ module Gitlab
         type == :ci
       end
 
-      def lfs_deploy_token?
-        type == :lfs_deploy_token
-      end
-
       def success?
         actor.present? || type == :ci
       end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
deleted file mode 100644
index d089a2f9b0b..00000000000
--- a/lib/gitlab/lfs_token.rb
+++ /dev/null
@@ -1,54 +0,0 @@
-module Gitlab
-  class LfsToken
-    attr_accessor :actor
-
-    TOKEN_LENGTH = 50
-    EXPIRY_TIME = 1800
-
-    def initialize(actor)
-      @actor =
-        case actor
-        when DeployKey, User
-          actor
-        when Key
-          actor.user
-        else
-          raise 'Bad Actor'
-        end
-    end
-
-    def generate
-      token = Devise.friendly_token(TOKEN_LENGTH)
-
-      Gitlab::Redis.with do |redis|
-        redis.set(redis_key, token, ex: EXPIRY_TIME)
-      end
-
-      token
-    end
-
-    def value
-      Gitlab::Redis.with do |redis|
-        redis.get(redis_key)
-      end
-    end
-
-    def user?
-      actor.is_a?(User)
-    end
-
-    def type
-      actor.is_a?(User) ? :lfs_token : :lfs_deploy_token
-    end
-
-    def actor_name
-      actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}"
-    end
-
-    private
-
-    def redis_key
-      "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor
-    end
-  end
-end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 21f0d46100e..8807a68a0a2 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -61,24 +61,6 @@ describe Gitlab::Auth, lib: true do
       expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities))
     end
 
-    it 'recognizes user lfs tokens' do
-      user = create(:user)
-      ip = 'ip'
-      token = Gitlab::LfsToken.new(user).generate
-
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, read_authentication_abilities))
-    end
-
-    it 'recognizes deploy key lfs tokens' do
-      key = create(:deploy_key)
-      ip = 'ip'
-      token = Gitlab::LfsToken.new(key).generate
-
-      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}")
-      expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities))
-    end
-
     it 'recognizes OAuth tokens' do
       user = create(:user)
       application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
deleted file mode 100644
index 9f04f67e0a8..00000000000
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ /dev/null
@@ -1,51 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::LfsToken, lib: true do
-  describe '#generate and #value' do
-    shared_examples 'an LFS token generator' do
-      it 'returns a randomly generated token' do
-        token = handler.generate
-
-        expect(token).not_to be_nil
-        expect(token).to be_a String
-        expect(token.length).to eq 50
-      end
-
-      it 'returns the correct token based on the key' do
-        token = handler.generate
-
-        expect(handler.value).to eq(token)
-      end
-    end
-
-    context 'when the actor is a user' do
-      let(:actor) { create(:user) }
-      let(:handler) { described_class.new(actor) }
-
-      it_behaves_like 'an LFS token generator'
-
-      it 'returns the correct username' do
-        expect(handler.actor_name).to eq(actor.username)
-      end
-
-      it 'returns the correct token type' do
-        expect(handler.type).to eq(:lfs_token)
-      end
-    end
-
-    context 'when the actor is a deploy key' do
-      let(:actor) { create(:deploy_key) }
-      let(:handler) { described_class.new(actor) }
-
-      it_behaves_like 'an LFS token generator'
-
-      it 'returns the correct username' do
-        expect(handler.actor_name).to eq("lfs+deploy-key-#{actor.id}")
-      end
-
-      it 'returns the correct token type' do
-        expect(handler.type).to eq(:lfs_deploy_token)
-      end
-    end
-  end
-end
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 46e8e6f1169..46d1b868782 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -100,43 +100,6 @@ describe API::API, api: true  do
     end
   end
 
-  describe "POST /internal/lfs_authenticate" do
-    before do
-      project.team << [user, :developer]
-    end
-
-    context 'user key' do
-      it 'returns the correct information about the key' do
-        lfs_auth(key.id, project)
-
-        expect(response).to have_http_status(200)
-        expect(json_response['username']).to eq(user.username)
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
-
-        expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
-      end
-
-      it 'returns a 404 when the wrong key is provided' do
-        lfs_auth(nil, project)
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'deploy key' do
-      let(:key) { create(:deploy_key) }
-
-      it 'returns the correct information about the key' do
-        lfs_auth(key.id, project)
-
-        expect(response).to have_http_status(200)
-        expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
-        expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
-      end
-    end
-  end
-
   describe "GET /internal/discover" do
     it do
       get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
@@ -426,13 +389,4 @@ describe API::API, api: true  do
       protocol: 'ssh'
     )
   end
-
-  def lfs_auth(key_id, project)
-    post(
-      api("/internal/lfs_authenticate"),
-      key_id: key_id,
-      secret_token: secret_token,
-      project: project.path_with_namespace
-    )
-  end
 end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 09e4e265dd1..b58d410b7a3 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -245,18 +245,6 @@ describe 'Git LFS API and storage' do
           end
         end
 
-        context 'when deploy key is authorized' do
-          let(:key) { create(:deploy_key) }
-          let(:authorization) { authorize_deploy_key }
-
-          let(:update_permissions) do
-            project.deploy_keys << key
-            project.lfs_objects << lfs_object
-          end
-
-          it_behaves_like 'responds with a file'
-        end
-
         context 'when build is authorized as' do
           let(:authorization) { authorize_ci_project }
 
@@ -1109,10 +1097,6 @@ describe 'Git LFS API and storage' do
     ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
   end
 
-  def authorize_deploy_key
-    ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
-  end
-
   def fork_project(project, user, object = nil)
     allow(RepositoryForkWorker).to receive(:perform_async).and_return(true)
     Projects::ForkService.new(project, user, {}).execute

From dc2968546500af4ea17dd23f851f00e002290bcc Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 13:50:28 +0200
Subject: [PATCH 40/41] Properly support Gitlab::Auth::Result

---
 lib/gitlab/auth.rb        | 14 +++++++-------
 lib/gitlab/auth/result.rb |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 7464d6082cb..0a0f1c3b17b 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -12,7 +12,7 @@ module Gitlab
           user_with_password_for_git(login, password) ||
           oauth_access_token_check(login, password) ||
           personal_access_token_check(login, password) ||
-          Result.new
+          Gitlab::Auth::Result.new
 
         rate_limit!(ip, success: result.success?, login: login)
 
@@ -70,7 +70,7 @@ module Gitlab
           service = project.public_send("#{underscored_service}_service")
 
           if service && service.activated? && service.valid_token?(password)
-            Result.new(nil, project, :ci, build_authentication_abilities)
+            Gitlab::Auth::Result.new(nil, project, :ci, build_authentication_abilities)
           end
         end
       end
@@ -81,7 +81,7 @@ module Gitlab
 
         raise Gitlab::Auth::MissingPersonalTokenError if user.two_factor_enabled?
 
-        Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
+        Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
       end
 
       def oauth_access_token_check(login, password)
@@ -89,7 +89,7 @@ module Gitlab
           token = Doorkeeper::AccessToken.by_token(password)
           if token && token.accessible?
             user = User.find_by(id: token.resource_owner_id)
-            Result.new(user, nil, :oauth, read_authentication_abilities)
+            Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities)
           end
         end
       end
@@ -98,7 +98,7 @@ module Gitlab
         if login && password
           user = User.find_by_personal_access_token(password)
           validation = User.by_login(login)
-          Result.new(user, nil, :personal_token, full_authentication_abilities) if user.present? && user == validation
+          Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities) if user.present? && user == validation
         end
       end
 
@@ -112,10 +112,10 @@ module Gitlab
 
         if build.user
           # If user is assigned to build, use restricted credentials of user
-          Result.new(build.user, build.project, :build, build_authentication_abilities)
+          Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)
         else
           # Otherwise use generic CI credentials (backward compatibility)
-          Result.new(nil, build.project, :ci, build_authentication_abilities)
+          Gitlab::Auth::Result.new(nil, build.project, :ci, build_authentication_abilities)
         end
       end
 
diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb
index bf625649cbf..3ec5765b6b0 100644
--- a/lib/gitlab/auth/result.rb
+++ b/lib/gitlab/auth/result.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
+    class Result < Struct.new(:actor, :project, :type, :authentication_abilities)
       def ci?
         type == :ci
       end

From 135be3cabb01ca3c825829f18ede4e8720383d7b Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 19 Sep 2016 14:23:18 +0200
Subject: [PATCH 41/41] Solve code review comments

---
 lib/gitlab/auth/result.rb           | 2 +-
 spec/requests/ci/api/builds_spec.rb | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb
index 3ec5765b6b0..bf625649cbf 100644
--- a/lib/gitlab/auth/result.rb
+++ b/lib/gitlab/auth/result.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Auth
-    class Result < Struct.new(:actor, :project, :type, :authentication_abilities)
+    Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
       def ci?
         type == :ci
       end
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index 09d72fe0a0e..df97f1bf7b6 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -263,6 +263,7 @@ describe Ci::API::API do
         context "should authorize posting artifact to running build" do
           it "using token as parameter" do
             post authorize_url, { token: build.token }, headers
+
             expect(response).to have_http_status(200)
             expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
             expect(json_response["TempPath"]).not_to be_nil
@@ -270,6 +271,7 @@ describe Ci::API::API do
 
           it "using token as header" do
             post authorize_url, {}, headers_with_token
+
             expect(response).to have_http_status(200)
             expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
             expect(json_response["TempPath"]).not_to be_nil
@@ -277,6 +279,7 @@ describe Ci::API::API do
 
           it "using runners token" do
             post authorize_url, { token: build.project.runners_token }, headers
+
             expect(response).to have_http_status(200)
             expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
             expect(json_response["TempPath"]).not_to be_nil
@@ -284,7 +287,9 @@ describe Ci::API::API do
 
           it "reject requests that did not go through gitlab-workhorse" do
             headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
+
             post authorize_url, { token: build.token }, headers
+
             expect(response).to have_http_status(500)
           end
         end
@@ -292,13 +297,17 @@ describe Ci::API::API do
         context "should fail to post too large artifact" do
           it "using token as parameter" do
             stub_application_setting(max_artifacts_size: 0)
+
             post authorize_url, { token: build.token, filesize: 100 }, headers
+
             expect(response).to have_http_status(413)
           end
 
           it "using token as header" do
             stub_application_setting(max_artifacts_size: 0)
+
             post authorize_url, { filesize: 100 }, headers_with_token
+
             expect(response).to have_http_status(413)
           end
         end