Merge branch 'ce-to-ee-2018-09-06' into 'master'

CE upstream - 2018-09-06 12:23 UTC

See merge request gitlab-org/gitlab-ee!7267

GITLAB_WORKHORSE_VERSION

-6.0.0
+6.1.0

Gemfile

@@ -110,7 +110,9 @@ gem 'kaminari', '~> 1.0'
 gem 'hamlit', '~> 2.8.8'
 
 # Files attachments
-gem 'carrierwave', '~> 1.2'
+# Locked until https://github.com/carrierwaveuploader/carrierwave/pull/2332/files is merged.
+# config/initializers/carrierwave_patch.rb can be removed once that change is released.
+gem 'carrierwave', '= 1.2.3'
 gem 'mini_magick'
 
 # Drag and Drop UI

Gemfile.lock

@@ -1026,7 +1026,7 @@ DEPENDENCIES
   bundler-audit (~> 0.5.0)
   capybara (~> 2.15)
   capybara-screenshot (~> 1.0.0)
-  carrierwave (~> 1.2)
+  carrierwave (= 1.2.3)
   charlock_holmes (~> 0.7.5)
   chronic (~> 0.10.2)
   chronic_duration (~> 0.10.6)

Gemfile.rails5.lock

@@ -1035,7 +1035,7 @@ DEPENDENCIES
   bundler-audit (~> 0.5.0)
   capybara (~> 2.15)
   capybara-screenshot (~> 1.0.0)
-  carrierwave (~> 1.2)
+  carrierwave (= 1.2.3)
   charlock_holmes (~> 0.7.5)
   chronic (~> 0.10.2)
   chronic_duration (~> 0.10.6)

app/assets/javascripts/diffs/components/changed_files_dropdown.vue

@@ -63,7 +63,7 @@ export default {
           v-else
           role="button"
           class="fa fa-times dropdown-input-search"
-          @click="clearSearch"
+          @click.stop.prevent="clearSearch"
         ></i>
       </div>
       <div class="dropdown-content">

app/assets/stylesheets/framework/awards.scss

@@ -229,8 +229,8 @@
 
     svg {
       margin-bottom: 1px;
-      height: 18px;
-      width: 18px;
+      height: $default-icon-size;
+      width: $default-icon-size;
       border-radius: 50%;
 
       path {

app/assets/stylesheets/framework/buttons.scss

@@ -149,7 +149,8 @@
   &.btn-success,
   &.btn-new,
   &.btn-create,
-  &.btn-save {
+  &.btn-save,
+  &.btn-register {
     @include btn-green;
   }
 
@@ -172,8 +173,7 @@
   }
 
   &.btn-info,
-  &.btn-primary,
-  &.btn-register {
+  &.btn-primary {
     @include btn-blue;
   }
 
@@ -248,7 +248,7 @@
 .btn-terminal {
   svg {
     height: 14px;
-    width: 18px;
+    width: $default-icon-size;
   }
 }
 

app/assets/stylesheets/framework/filters.scss

@@ -216,8 +216,8 @@
     vertical-align: inherit;
 
     img {
-      height: 18px;
-      width: 18px;
+      height: $default-icon-size;
+      width: $default-icon-size;
     }
   }
 

app/assets/stylesheets/framework/toggle.scss

@@ -56,8 +56,8 @@
 
     &,
     .toggle-icon-svg {
-      width: 18px;
-      height: 18px;
+      width: $default-icon-size;
+      height: $default-icon-size;
     }
 
     .toggle-icon-svg {

app/assets/stylesheets/framework/variables.scss

@@ -252,7 +252,7 @@ $container-text-max-width: 540px;
 $gl-avatar-size: 40px;
 $border-radius-default: 4px;
 $border-radius-small: 2px;
-$settings-icon-size: 18px;
+$default-icon-size: 18px;
 $layout-link-gray: #7e7c7c;
 $btn-side-margin: 10px;
 $btn-sm-side-margin: 7px;

app/assets/stylesheets/pages/diff.scss

@@ -749,6 +749,10 @@
     left: $gl-padding;
   }
 
+  .dropdown-input .dropdown-input-search {
+    pointer-events: all;
+  }
+
   .diff-changed-file {
     display: flex;
     padding-top: 8px;

app/assets/stylesheets/pages/login.scss

@@ -100,6 +100,22 @@
     p {
       margin: 0;
     }
+
+    .omniauth-btn {
+      margin-bottom: $gl-padding;
+      width: 48%;
+      padding: $gl-padding-8;
+
+      @include media-breakpoint-down(md) {
+        width: 100%;
+      }
+
+      img {
+        width: $default-icon-size;
+        height: $default-icon-size;
+        margin-right: $gl-padding;
+      }
+    }
   }
 
   .new-session-tabs {
@@ -169,10 +185,6 @@
     }
   }
 
-  label {
-    font-weight: $gl-font-weight-normal;
-  }
-
   .submit-container {
     margin-top: 16px;
   }
@@ -200,15 +212,6 @@
   }
 }
 
-.oauth-image-link {
-  margin-right: 10px;
-
-  img {
-    width: 32px;
-    height: 32px;
-  }
-}
-
 .devise-layout-html {
   margin: 0;
   padding: 0;

app/assets/stylesheets/pages/settings.scss

@@ -106,7 +106,7 @@
 
 .settings-list-icon {
   color: $gl-text-color-secondary;
-  font-size: $settings-icon-size;
+  font-size: $default-icon-size;
   line-height: 42px;
 }
 

app/controllers/concerns/send_file_upload.rb

 module SendFileUpload
   def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, disposition: 'attachment')
     if attachment
-      redirect_params[:query] = { "response-content-disposition" => "#{disposition};filename=#{attachment.inspect}" }
+      # Response-Content-Type will not override an existing Content-Type in
+      # Google Cloud Storage, so the metadata needs to be cleared on GCS for
+      # this to work. However, this override works with AWS.
+      redirect_params[:query] = { "response-content-disposition" => "#{disposition};filename=#{attachment.inspect}",
+                                  "response-content-type" => guess_content_type(attachment) }
       # By default, Rails will send uploads with an extension of .js with a
       # content-type of text/javascript, which will trigger Rails'
       # cross-origin JavaScript protection.
@@ -18,4 +22,14 @@ module SendFileUpload
       redirect_to file_upload.url(**redirect_params)
     end
   end
+
+  def guess_content_type(filename)
+    types = MIME::Types.type_for(filename)
+
+    if types.present?
+      types.first.content_type
+    else
+      "application/octet-stream"
+    end
+  end
 end

app/serializers/build_details_entity.rb

@@ -36,6 +36,10 @@ class BuildDetailsEntity < JobEntity
     erase_project_job_path(project, build)
   end
 
+  expose :terminal_path, if: -> (*) { can_create_build_terminal? } do |build|
+    terminal_project_job_path(project, build)
+  end
+
   expose :merge_request, if: -> (*) { can?(current_user, :read_merge_request, build.merge_request) } do
     expose :iid do |build|
       build.merge_request.iid
@@ -69,4 +73,8 @@ class BuildDetailsEntity < JobEntity
   def project
     build.project
   end
+
+  def can_create_build_terminal?
+    can?(current_user, :create_build_terminal, build) && build.has_terminal?
+  end
 end

app/views/devise/sessions/_new_base.html.haml

 = form_for(resource, as: resource_name, url: session_path(resource_name), html: { class: 'new_user gl-show-field-errors', 'aria-live' => 'assertive'}) do |f|
   .form-group
-    = f.label "Username or email", for: "user_login"
+    = f.label "Username or email", for: "user_login", class: 'label-bold'
     = f.text_field :login, class: "form-control top", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required."
   .form-group
-    = f.label :password
+    = f.label :password, class: 'label-bold'
     = f.password_field :password, class: "form-control bottom", required: true, title: "This field is required."
   - if devise_mapping.rememberable?
     .remember-me

app/views/devise/shared/_omniauth_box.html.haml

-.omniauth-container
-  %p
-    %span.light
-      Sign in with  
-    - providers = enabled_button_based_providers
+.omniauth-container.prepend-top-15
+  %label.label-bold.d-block
+    Sign in with
+  - providers = enabled_button_based_providers
+  .d-flex.justify-content-between.flex-wrap
     - providers.each do |provider|
-      %span.light
-        - has_icon = provider_has_icon?(provider)
-        = link_to provider_image_tag(provider), omniauth_authorize_path(:user, provider), method: :post, class: 'oauth-login' + (has_icon ? ' oauth-image-link' : ' btn'), id: "oauth-login-#{provider}"
-    %fieldset.prepend-top-10.remember-me
-      %label
-        = check_box_tag :remember_me, nil, false, class: 'remember-me-checkbox'
+      - has_icon = provider_has_icon?(provider)
+      = link_to omniauth_authorize_path(:user, provider), method: :post, class: 'btn d-flex align-items-center omniauth-btn text-left oauth-login', id: "oauth-login-#{provider}" do
+        - if has_icon
+          = provider_image_tag(provider)
         %span
-          Remember me
+          = label_for_provider(provider)
+  %fieldset.remember-me
+    %label
+      = check_box_tag :remember_me, nil, false, class: 'remember-me-checkbox'
+      %span
+        Remember me

app/views/devise/shared/_signup_box.html.haml

@@ -4,24 +4,24 @@
       .devise-errors
         = devise_error_messages!
       .form-group
-        = f.label :name, 'Full name'
+        = f.label :name, 'Full name', class: 'label-bold'
         = f.text_field :name, class: "form-control top", required: true, title: "This field is required."
       .username.form-group
-        = f.label :username
+        = f.label :username, class: 'label-bold'
         = f.text_field :username, class: "form-control middle", pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: 'Please create a username with only alphanumeric characters.'
         %p.validation-error.hide Username is already taken.
         %p.validation-success.hide Username is available.
         %p.validation-pending.hide Checking username availability...
       .form-group
-        = f.label :email
+        = f.label :email, class: 'label-bold'
         = f.email_field :email, class: "form-control middle", required: true, title: "Please provide a valid email address."
       .form-group
-        = f.label :email_confirmation
+        = f.label :email_confirmation, class: 'label-bold'
         = f.email_field :email_confirmation, class: "form-control middle", required: true, title: "Please retype the email address."
       .form-group.append-bottom-20#password-strength
-        = f.label :password
+        = f.label :password, class: 'label-bold'
         = f.password_field :password, class: "form-control bottom", required: true, pattern: ".{#{@minimum_password_length},}", title: "Minimum length is #{@minimum_password_length} characters."
-        %p.gl-field-hint Minimum length is #{@minimum_password_length} characters
+        %p.gl-field-hint.text-secondary Minimum length is #{@minimum_password_length} characters
       - if Gitlab::CurrentSettings.current_application_settings.enforce_terms?
         .form-group
           = check_box_tag :terms_opt_in, '1', false, required: true
@@ -35,8 +35,3 @@
         = recaptcha_tags
       .submit-container
         = f.submit "Register", class: "btn-register btn"
-.clearfix.submit-container
-  %p
-    %span.light Didn't receive a confirmation email?
-    = succeed '.' do
-      = link_to "Request a new one", new_confirmation_path(:user)

app/views/profiles/show.html.haml

@@ -77,7 +77,7 @@
           = f.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9' },
           help: "Your name was automatically set based on your #{ attribute_provider_label(:name) } account, so people you know can recognize you."
         - else
-          = f.text_field :name, required: true, wrapper: { class: 'col-md-9' }, help: "Enter your name, so people you know can recognize you."
+          = f.text_field :name, label: 'Full name', required: true, wrapper: { class: 'col-md-9' }, help: "Enter your name, so people you know can recognize you."
         = f.text_field :id, readonly: true, label: 'User ID', wrapper: { class: 'col-md-3' }
 
       - if @user.read_only_attribute?(:email)

app/views/projects/blob/_blob.html.haml

@@ -4,6 +4,7 @@
   .well-segment
     %ul.blob-commit-info
       = render 'projects/commits/commit', commit: @last_commit, project: @project, ref: @ref
+
   = render_if_exists 'projects/blob/owners', blob: blob
   = render "projects/blob/auxiliary_viewer", blob: blob
 

changelogs/unreleased/44943-update-presentation-for-sso-providers-on-log-in-page.yml

+---
+title: Update presentation for SSO providers on log in page
+merge_request: 21233
+author:
+type: other

changelogs/unreleased/51092-fix-mr-diff-file-filter-clear-button.yml

+---
+title: Make MR diff file filter input Clear button functional
+merge_request: 21556
+author:
+type: fixed

changelogs/unreleased/51117-send-terminal-path-in-job-api.yml

+---
+title: Add terminal_path to job API response
+merge_request: 21537
+author:
+type: other

changelogs/unreleased/ash-mckenzie-geo-git-push-ssh-proxy.yml

+---
+title: 'Support a custom action, such as proxying to another server, after /api/v4/internal/allowed check succeeds'
+merge_request: 21034
+author:
+type: changed

changelogs/unreleased/feature-git-v2-flag.yml

+---
+title: Add git_v2 feature flag
+merge_request: 21520
+author:
+type: added

changelogs/unreleased/sh-fix-attachments-inline.yml

+---
+title: Fix attachments not displaying inline with Google Cloud Storage
+merge_request: 21265
+author:
+type: fixed

config/initializers/carrierwave_patch.rb

+# This monkey patches CarrierWave 1.2.3 to make Google Cloud Storage work with
+# extra query parameters:
+# https://github.com/carrierwaveuploader/carrierwave/pull/2332/files
+module CarrierWave
+  module Storage
+    class Fog < Abstract
+      class File
+        def authenticated_url(options = {})
+          if %w(AWS Google Rackspace OpenStack).include?(@uploader.fog_credentials[:provider])
+            # avoid a get by using local references
+            local_directory = connection.directories.new(key: @uploader.fog_directory)
+            local_file = local_directory.files.new(key: path)
+            expire_at = ::Fog::Time.now + @uploader.fog_authenticated_url_expiration
+            case @uploader.fog_credentials[:provider]
+            when 'AWS', 'Google'
+              local_file.url(expire_at, options)
+            when 'Rackspace'
+              connection.get_object_https_url(@uploader.fog_directory, path, expire_at, options)
+            when 'OpenStack'
+              connection.get_object_https_url(@uploader.fog_directory, path, expire_at)
+            else
+              local_file.url(expire_at)
+            end
+          end
+        end
+      end
+    end
+  end
+end

config/initializers/fog_google_https_private_urls.rb

@@ -9,7 +9,7 @@ module Fog
         module MonkeyPatch
           def url(expires, options = {})
             requires :key
-            collection.get_https_url(key, expires)
+            collection.get_https_url(key, expires, options)
           end
         end
 

lib/api/internal.rb

@@ -6,8 +6,17 @@ module API
     helpers ::API::Helpers::InternalHelpers
     helpers ::Gitlab::Identifier
 
+    UNKNOWN_CHECK_RESULT_ERROR = 'Unknown check result'.freeze
+
+    helpers do
+      def response_with_status(code: 200, success: true, message: nil, **extra_options)
+        status code
+        { status: success, message: message }.merge(extra_options).compact
+      end
+    end
+
     namespace 'internal' do
-      # Check if git command is allowed to project
+      # Check if git command is allowed for project
       #
       # Params:
       #   key_id - ssh key id for Git over SSH
@@ -18,8 +27,6 @@ module API
       #   action - git action (git-upload-pack or git-receive-pack)
       #   changes - changes as "oldrev newrev ref", see Gitlab::ChangesList
       post "/allowed" do
-        status 200
-
         # Stores some Git-specific env thread-safely
         env = parse_env
         Gitlab::Git::HookEnv.set(gl_repository, env) if project
@@ -49,27 +56,37 @@ module API
                     namespace_path: namespace_path, project_path: project_path,
                     redirected_path: redirected_path)
 
-        begin
-          access_checker.check(params[:action], params[:changes])
-          @project ||= access_checker.project
-        rescue Gitlab::GitAccess::UnauthorizedError, Gitlab::GitAccess::NotFoundError => e
-          break { status: false, message: e.message }
-        end
+        check_result = begin
+                         result = access_checker.check(params[:action], params[:changes])
+                         @project ||= access_checker.project
+                         result
+                       rescue Gitlab::GitAccess::UnauthorizedError => e
+                         break response_with_status(code: 401, success: false, message: e.message)
+                       rescue Gitlab::GitAccess::NotFoundError => e
+                         break response_with_status(code: 404, success: false, message: e.message)
+                       end
 
         log_user_activity(actor)
 
-        {
-          status: true,
-          gl_repository: gl_repository,
-          gl_id: Gitlab::GlId.gl_id(user),
-          gl_username: user&.username,
-
-          # This repository_path is a bogus value but gitlab-shell still requires
-          # its presence. https://gitlab.com/gitlab-org/gitlab-shell/issues/135
-          repository_path: '/',
-
-          gitaly: gitaly_payload(params[:action])
-        }
+        case check_result
+        when ::Gitlab::GitAccessResult::Success
+          payload = {
+            gl_repository: gl_repository,
+            gl_id: Gitlab::GlId.gl_id(user),
+            gl_username: user&.username,
+
+            # This repository_path is a bogus value but gitlab-shell still requires
+            # its presence. https://gitlab.com/gitlab-org/gitlab-shell/issues/135
+            repository_path: '/',
+
+            gitaly: gitaly_payload(params[:action])
+          }
+          response_with_status(**payload)
+        when ::Gitlab::GitAccessResult::CustomAction
+          response_with_status(code: 300, message: check_result.message, payload: check_result.payload)
+        else
+          response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR)
+        end
       end
 
       post "/lfs_authenticate" do

lib/gitlab/git_access.rb

@@ -53,6 +53,10 @@ module Gitlab
       check_authentication_abilities!(cmd)
       check_command_disabled!(cmd)
       check_command_existence!(cmd)
+
+      custom_action = check_custom_action(cmd)
+      return custom_action if custom_action
+
       check_db_accessibility!(cmd)
 
       ensure_project_on_push!(cmd, changes)
@@ -68,7 +72,7 @@ module Gitlab
         check_push_access!
       end
 
-      true
+      ::Gitlab::GitAccessResult::Success.new
     end
 
     def guest_can_download_code?
@@ -95,6 +99,10 @@ module Gitlab
 
     private
 
+    def check_custom_action(cmd)
+      nil
+    end
+
     def check_valid_actor!
       return unless actor.is_a?(Key)
 

lib/gitlab/git_access_result/custom_action.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GitAccessResult
+    class CustomAction
+      attr_reader :payload, :message
+
+      # Example of payload:
+      #
+      # {
+      #   'action' => 'geo_proxy_to_primary',
+      #   'data' => {
+      #     'api_endpoints' => %w{geo/proxy_git_push_ssh/info_refs geo/proxy_git_push_ssh/push},
+      #     'gl_username' => user.username,
+      #     'primary_repo' => geo_primary_http_url_to_repo(project_or_wiki)
+      #   }
+      # }
+      #
+      def initialize(payload, message)
+        @payload = payload
+        @message = message
+      end
+    end
+  end
+end

lib/gitlab/git_access_result/success.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GitAccessResult
+    class Success
+    end
+  end
+end

lib/gitlab/gitaly_client.rb

@@ -206,7 +206,7 @@ module Gitlab
       result
     end
 
-    SERVER_FEATURE_FLAGS = %w[gogit_findcommit].freeze
+    SERVER_FEATURE_FLAGS = %w[gogit_findcommit git_v2].freeze
 
     def self.server_feature_flags
       SERVER_FEATURE_FLAGS.map do |f|

lib/object_storage/direct_upload.rb

@@ -158,7 +158,7 @@ module ObjectStorage
     end
 
     def upload_options
-      { 'Content-Type' => 'application/octet-stream' }
+      {}
     end
 
     def connection

spec/controllers/concerns/send_file_upload_spec.rb

@@ -52,7 +52,7 @@ describe SendFileUpload do
     end
 
     context 'with attachment' do
-      subject { controller.send_upload(uploader, attachment: 'test.js') }
+      let(:send_attachment) { controller.send_upload(uploader, attachment: 'test.js') }
 
       it 'sends a file with content-type of text/plain' do
         expected_params = {
@@ -62,7 +62,29 @@ describe SendFileUpload do
         }
         expect(controller).to receive(:send_file).with(uploader.path, expected_params)
 
-        subject
+        send_attachment
+      end
+
+      context 'with a proxied file in object storage' do
+        before do
+          stub_uploads_object_storage(uploader: uploader_class)
+          uploader.object_store = ObjectStorage::Store::REMOTE
+          uploader.store!(temp_file)
+          allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true }
+        end
+
+        it 'sends a file with a custom type' do
+          headers = double
+          expected_headers = %r(response-content-disposition=attachment%3Bfilename%3D%22test.js%22&response-content-type=application/javascript)
+          expect(Gitlab::Workhorse).to receive(:send_url).with(expected_headers).and_call_original
+          expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/)
+
+          expect(controller).not_to receive(:send_file)
+          expect(controller).to receive(:headers) { headers }
+          expect(controller).to receive(:head).with(:ok)
+
+          send_attachment
+        end
       end
     end
 
@@ -80,7 +102,12 @@ describe SendFileUpload do
 
         it 'sends a file' do
           headers = double
+          expect(Gitlab::Workhorse).not_to receive(:send_url).with(/response-content-disposition/)
+          expect(Gitlab::Workhorse).not_to receive(:send_url).with(/response-content-type/)
+          expect(Gitlab::Workhorse).to receive(:send_url).and_call_original
+
           expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/)
+          expect(controller).not_to receive(:send_file)
           expect(controller).to receive(:headers) { headers }
           expect(controller).to receive(:head).with(:ok)
 

spec/controllers/projects/jobs_controller_spec.rb

@@ -135,7 +135,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       end
     end
 
-    context 'when requesting JSON with failed job' do
+    context 'when requesting JSON' do
       let(:merge_request) { create(:merge_request, source_project: project) }
 
       before do
@@ -147,61 +147,51 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
         get_show(id: job.id, format: :json)
       end
 
-      it 'exposes needed information' do
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to match_response_schema('job/job_details')
-        expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z})
-        expect(json_response['merge_request']['path']).to match(%r{merge_requests/\d+\z})
-        expect(json_response['new_issue_path']).to include('/issues/new')
+      context 'when job failed' do
+        it 'exposes needed information' do
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('job/job_details')
+          expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z})
+          expect(json_response.dig('merge_request', 'path')).to match(%r{merge_requests/\d+\z})
+          expect(json_response['new_issue_path']).to include('/issues/new')
+        end
       end
-    end
-
-    context 'when request JSON for successful job' do
-      let(:merge_request) { create(:merge_request, source_project: project) }
-      let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
-
-      before do
-        project.add_developer(user)
-        sign_in(user)
 
-        allow_any_instance_of(Ci::Build).to receive(:merge_request).and_return(merge_request)
+      context 'when job has artifacts' do
+        context 'with not expiry date' do
+          let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
 
-        get_show(id: job.id, format: :json)
-      end
+          it 'exposes needed information' do
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to match_response_schema('job/job_details')
+            expect(json_response['artifact']['download_path']).to match(%r{artifacts/download})
+            expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse})
+            expect(json_response['artifact']).not_to have_key(:expired)
+            expect(json_response['artifact']).not_to have_key(:expired_at)
+          end
+        end
 
-      it 'exposes needed information' do
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to match_response_schema('job/job_details')
-        expect(json_response['artifact']['download_path']).to match(%r{artifacts/download})
-        expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse})
-        expect(json_response['artifact']).not_to have_key(:expired)
-        expect(json_response['artifact']).not_to have_key(:expired_at)
-        expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z})
-        expect(json_response.dig('merge_request', 'path')).to match(%r{merge_requests/\d+\z})
+        context 'with expiry date' do
+          let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) }
+
+          it 'exposes needed information' do
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to match_response_schema('job/job_details')
+            expect(json_response['artifact']).not_to have_key(:download_path)
+            expect(json_response['artifact']).not_to have_key(:browse_path)
+            expect(json_response['artifact']['expired']).to eq(true)
+            expect(json_response['artifact']['expire_at']).not_to be_empty
+          end
+        end
       end
 
-      context 'when request JSON for successful job with expired artifacts' do
-        let(:merge_request) { create(:merge_request, source_project: project) }
-        let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) }
-
-        before do
-          project.add_developer(user)
-          sign_in(user)
-
-          allow_any_instance_of(Ci::Build).to receive(:merge_request).and_return(merge_request)
-
-          get_show(id: job.id, format: :json)
-        end
+      context 'when job has terminal' do
+        let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) }
 
-        it 'exposes needed information' do
+        it 'exposes the terminal path' do
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to match_response_schema('job/job_details')
-          expect(json_response['artifact']).not_to have_key(:download_path)
-          expect(json_response['artifact']).not_to have_key(:browse_path)
-          expect(json_response['artifact']['expired']).to eq(true)
-          expect(json_response['artifact']['expire_at']).not_to be_empty
-          expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z})
-          expect(json_response.dig('merge_request', 'path')).to match(%r{merge_requests/\d+\z})
+          expect(json_response['terminal_path']).to match(%r{/terminal})
         end
       end
     end

spec/fixtures/api/schemas/job/job_details.json

@@ -2,6 +2,7 @@
   "allOf": [{ "$ref": "job.json" }],
   "description": "An extension of job.json with more detailed information",
   "properties": {
-    "artifact": { "$ref": "artifact.json" }
+    "artifact": { "$ref": "artifact.json" },
+    "terminal_path": { "type": "string" }
   }
 }

spec/lib/object_storage/direct_upload_spec.rb

@@ -62,7 +62,7 @@ describe ObjectStorage::DirectUpload do
         expect(subject[:StoreURL]).to start_with(storage_url)
         expect(subject[:DeleteURL]).to start_with(storage_url)
         expect(subject[:CustomPutHeaders]).to be_truthy
-        expect(subject[:PutHeaders]).to eq({ 'Content-Type' => 'application/octet-stream' })
+        expect(subject[:PutHeaders]).to eq({})
       end
     end
 

spec/requests/api/internal_spec.rb

@@ -381,7 +381,7 @@ describe API::Internal do
         it do
           pull(key, project)
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(401)
           expect(json_response["status"]).to be_falsey
           expect(user.reload.last_activity_on).to be_nil
         end
@@ -391,13 +391,61 @@ describe API::Internal do
         it do
           push(key, project)
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(401)
           expect(json_response["status"]).to be_falsey
           expect(user.reload.last_activity_on).to be_nil
         end
       end
     end
 
+    context "custom action" do
+      let(:access_checker) { double(Gitlab::GitAccess) }
+      let(:message) { 'CustomActionError message' }
+      let(:payload) do
+        {
+          'action' => 'geo_proxy_to_primary',
+          'data' => {
+            'api_endpoints' => %w{geo/proxy_git_push_ssh/info_refs geo/proxy_git_push_ssh/push},
+            'gl_username' => 'testuser',
+            'primary_repo' => 'http://localhost:3000/testuser/repo.git'
+          }
+        }
+      end
+
+      let(:custom_action_result) { Gitlab::GitAccessResult::CustomAction.new(payload, message) }
+
+      before do
+        project.add_guest(user)
+        expect(Gitlab::GitAccess).to receive(:new).with(
+          key,
+          project,
+          'ssh',
+          {
+            authentication_abilities: [:read_project, :download_code, :push_code],
+            namespace_path: project.namespace.name,
+            project_path: project.path,
+            redirected_path: nil
+          }
+        ).and_return(access_checker)
+        expect(access_checker).to receive(:check).with(
+          'git-receive-pack',
+          'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master'
+        ).and_return(custom_action_result)
+      end
+
+      context "git push" do
+        it do
+          push(key, project)
+
+          expect(response).to have_gitlab_http_status(300)
+          expect(json_response['status']).to be_truthy
+          expect(json_response['message']).to eql(message)
+          expect(json_response['payload']).to eql(payload)
+          expect(user.reload.last_activity_on).to be_nil
+        end
+      end
+    end
+
     context "blocked user" do
       let(:personal_project) { create(:project, namespace: user.namespace) }
 
@@ -409,7 +457,7 @@ describe API::Internal do
         it do
           pull(key, personal_project)
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(401)
           expect(json_response["status"]).to be_falsey
           expect(user.reload.last_activity_on).to be_nil
         end
@@ -419,7 +467,7 @@ describe API::Internal do
         it do
           push(key, personal_project)
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(401)
           expect(json_response["status"]).to be_falsey
           expect(user.reload.last_activity_on).to be_nil
         end
@@ -445,7 +493,7 @@ describe API::Internal do
         it do
           push(key, project)
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(401)
           expect(json_response["status"]).to be_falsey
         end
       end
@@ -477,7 +525,7 @@ describe API::Internal do
         it do
           archive(key, project)
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(404)
           expect(json_response["status"]).to be_falsey
         end
       end
@@ -489,7 +537,7 @@ describe API::Internal do
 
         pull(key, project)
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to have_gitlab_http_status(404)
         expect(json_response["status"]).to be_falsey
       end
     end
@@ -498,7 +546,7 @@ describe API::Internal do
       it do
         pull(OpenStruct.new(id: 0), project)
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to have_gitlab_http_status(404)
         expect(json_response["status"]).to be_falsey
       end
     end
@@ -511,7 +559,7 @@ describe API::Internal do
       it 'rejects the SSH push' do
         push(key, project)
 
-        expect(response.status).to eq(200)
+        expect(response.status).to eq(401)
         expect(json_response['status']).to be_falsey
         expect(json_response['message']).to eq 'Git access over SSH is not allowed'
       end
@@ -519,7 +567,7 @@ describe API::Internal do
       it 'rejects the SSH pull' do
         pull(key, project)
 
-        expect(response.status).to eq(200)
+        expect(response.status).to eq(401)
         expect(json_response['status']).to be_falsey
         expect(json_response['message']).to eq 'Git access over SSH is not allowed'
       end
@@ -533,7 +581,7 @@ describe API::Internal do
       it 'rejects the HTTP push' do
         push(key, project, 'http')
 
-        expect(response.status).to eq(200)
+        expect(response.status).to eq(401)
         expect(json_response['status']).to be_falsey
         expect(json_response['message']).to eq 'Git access over HTTP is not allowed'
       end
@@ -541,7 +589,7 @@ describe API::Internal do
       it 'rejects the HTTP pull' do
         pull(key, project, 'http')
 
-        expect(response.status).to eq(200)
+        expect(response.status).to eq(401)
         expect(json_response['status']).to be_falsey
         expect(json_response['message']).to eq 'Git access over HTTP is not allowed'
       end
@@ -571,14 +619,14 @@ describe API::Internal do
       it 'rejects the push' do
         push(key, project)
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to have_gitlab_http_status(404)
         expect(json_response['status']).to be_falsy
       end
 
       it 'rejects the SSH pull' do
         pull(key, project)
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to have_gitlab_http_status(404)
         expect(json_response['status']).to be_falsy
       end
     end