Merge branch 'fj-5276-mirror-webide-changes' into 'master'

Implement endpoint to perform request to services running in builds

Closes #9538 and #5276

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

app/controllers/projects/jobs_controller.rb

@@ -208,3 +208,5 @@ class Projects::JobsController < Projects::ApplicationController
     'attachment'
   end
 end
+
+Projects::JobsController.prepend(EE::Projects::JobsController)

app/models/ci/build_runner_session.rb

@@ -37,3 +37,5 @@ module Ci
     end
   end
 end
+
+Ci::BuildRunnerSession.prepend(EE::Ci::BuildRunnerSession)

ee/app/controllers/ee/projects/jobs_controller.rb

+# frozen_string_literal: true
+
+module EE
+  module Projects
+    module JobsController
+      extend ActiveSupport::Concern
+
+      prepended do
+        before_action :authorize_create_proxy_build!, only: :proxy_websocket_authorize
+        before_action :verify_proxy_request!, only: :proxy_websocket_authorize
+      end
+
+      def proxy_websocket_authorize
+        render json: proxy_websocket_service(build_service_specification)
+      end
+
+      private
+
+      def authorize_create_proxy_build!
+        return access_denied! unless can?(current_user, :create_build_service_proxy, build)
+      end
+
+      def verify_proxy_request!
+        ::Gitlab::Workhorse.verify_api_request!(request.headers)
+        set_workhorse_internal_api_content_type
+      end
+
+      # This method provides the information to Workhorse
+      # about the service we want to proxy to.
+      # For security reasons, in case this operation is started by JS,
+      # it's important to use only sourced GitLab JS code
+      def proxy_websocket_service(service)
+        service[:url] = ::Gitlab::UrlHelpers.as_wss(service[:url])
+
+        ::Gitlab::Workhorse.channel_websocket(service)
+      end
+
+      def build_service_specification
+        build.service_specification(service: params['service'],
+                                    port: params['port'],
+                                    path: params['path'],
+                                    subprotocols: proxy_subprotocol)
+      end
+
+      def proxy_subprotocol
+        # This will allow to reuse the same subprotocol set
+        # in the original websocket connection
+        request.headers['HTTP_SEC_WEBSOCKET_PROTOCOL'].presence || ::Ci::BuildRunnerSession::TERMINAL_SUBPROTOCOL
+      end
+    end
+  end
+end

ee/app/models/ee/ci/build.rb

@@ -18,6 +18,7 @@ module EE
 
       prepended do
         after_save :stick_build_if_status_changed
+        delegate :service_specification, to: :runner_session, allow_nil: true
 
         has_many :sourced_pipelines,
           class_name: ::Ci::Sources::Pipeline,

ee/app/models/ee/ci/build_runner_session.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    module BuildRunnerSession
+      extend ActiveSupport::Concern
+
+      DEFAULT_SERVICE_NAME = 'build'.freeze
+      DEFAULT_PORT_NAME = 'default_port'.freeze
+
+      def service_specification(service: nil, path: nil, port: nil, subprotocols: nil)
+        return {} unless url.present?
+
+        port = port.presence || DEFAULT_PORT_NAME
+        service = service.presence || DEFAULT_SERVICE_NAME
+        url = "#{self.url}/proxy/#{service}/#{port}/#{path}"
+        subprotocols = subprotocols.presence || ::Ci::BuildRunnerSession::TERMINAL_SUBPROTOCOL
+
+        channel_specification(url, subprotocols)
+      end
+    end
+  end
+end

ee/app/models/web_ide_terminal.rb

@@ -28,14 +28,20 @@ class WebIdeTerminal
     terminal_project_job_path(project, build, format: :ws)
   end
 
+  def proxy_websocket_path
+    proxy_project_job_path(project, build, format: :ws)
+  end
+
   private
 
-  def web_ide_terminal_route_generator(action)
-    url_for(action: action,
-            controller: 'projects/web_ide_terminals',
-            namespace_id: project.namespace.to_param,
-            project_id: project.to_param,
-            id: build.id,
-            only_path: true)
+  def web_ide_terminal_route_generator(action, options = {})
+    options.reverse_merge!(action: action,
+                           controller: 'projects/web_ide_terminals',
+                           namespace_id: project.namespace.to_param,
+                           project_id: project.to_param,
+                           id: build.id,
+                           only_path: true)
+
+    url_for(options)
   end
 end

ee/app/policies/ee/ci/build_policy.rb

@@ -26,6 +26,11 @@ module EE
 
         rule { can?(:update_web_ide_terminal) & terminal }.policy do
           enable :create_build_terminal
+          enable :create_build_service_proxy
+        end
+
+        rule { ~can?(:build_service_proxy_enabled) }.policy do
+          prevent :create_build_service_proxy
         end
 
         private

ee/app/policies/ee/project_policy.rb

@@ -192,6 +192,10 @@ module EE
         @subject.feature_available?(:web_ide_terminal)
       end
 
+      condition(:build_service_proxy_enabled) do
+        ::Feature.enabled?(:build_service_proxy, @subject)
+      end
+
       rule { web_ide_terminal_available & can?(:create_pipeline) & can?(:maintainer_access) }.enable :create_web_ide_terminal
 
       # Design abilities could also be prevented in the issue policy.
@@ -201,6 +205,8 @@ module EE
         prevent :create_design
         prevent :destroy_design
       end
+
+      rule { build_service_proxy_enabled }.enable :build_service_proxy_enabled
     end
   end
 end

ee/app/serializers/web_ide_terminal_entity.rb

@@ -7,4 +7,5 @@ class WebIdeTerminalEntity < Grape::Entity
   expose :cancel_path
   expose :retry_path
   expose :terminal_path
+  expose :proxy_websocket_path, if: ->(_) { Feature.enabled?(:build_service_proxy) }
 end

ee/changelogs/unreleased/fj-5276-mirror-webide-changes.yml

+---
+title: Proxy websocket requests to build services
+merge_request: 9723
+author:
+type: added

ee/config/routes/project.rb

@@ -47,6 +47,13 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
             get :download
           end
         end
+
+        resources :jobs, only: [], constraints: { id: /\d+/ } do
+          member do
+            get '/proxy.ws/authorize', to: 'jobs#proxy_websocket_authorize', constraints: { format: nil }
+            get :proxy
+          end
+        end
       end
 
       namespace :settings do

ee/spec/controllers/ee/projects/jobs_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::JobsController do
+  let(:owner) { create(:owner) }
+  let(:admin) { create(:admin) }
+  let(:maintainer) { create(:user) }
+  let(:developer) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:guest) { create(:user) }
+  let(:project) { create(:project, :private, :repository, namespace: owner.namespace) }
+  let(:pipeline) { create(:ci_pipeline, project: project, source: :webide, config_source: :webide_source, user: user) }
+  let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline, user: user) }
+  let(:user) { maintainer }
+  let(:extra_params) { { id: job.id } }
+
+  before do
+    stub_licensed_features(web_ide_terminal: true)
+    stub_feature_flags(build_service_proxy: true)
+    allow(job).to receive(:has_terminal?).and_return(true)
+
+    project.add_maintainer(maintainer)
+    project.add_developer(developer)
+    project.add_reporter(reporter)
+    project.add_guest(guest)
+
+    sign_in(user)
+  end
+
+  shared_examples 'proxy access rights' do
+    before do
+      allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
+
+      make_request
+    end
+
+    context 'with admin' do
+      let(:user) { admin }
+
+      it 'returns 200' do
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
+
+    context 'with owner' do
+      let(:user) { owner }
+
+      it 'returns 200' do
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
+
+    context 'with maintainer' do
+      let(:user) { maintainer }
+
+      it 'returns 200' do
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
+
+    context 'with developer' do
+      let(:user) { developer }
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'with reporter' do
+      let(:user) { reporter }
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'with guest' do
+      let(:user) { guest }
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'with non member' do
+      let(:user) { create(:user) }
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  shared_examples 'when pipeline is not from a webide source' do
+    context 'with admin' do
+      let(:user) { admin }
+      let(:pipeline) { create(:ci_pipeline, project: project, source: :chat, user: user) }
+
+      before do
+        allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
+        make_request
+      end
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  shared_examples 'validates workhorse signature' do
+    context 'with valid workhorse signature' do
+      before do
+        allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
+      end
+
+      context 'and valid id' do
+        it 'returns the proxy data for the service running in the job' do
+          make_request
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.headers["Content-Type"]).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+          expect(response.body).to eq(expected_data)
+        end
+      end
+
+      context 'and invalid id' do
+        let(:extra_params) { { id: 1234 } }
+
+        it 'returns 404' do
+          make_request
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
+
+    context 'with invalid workhorse signature' do
+      it 'aborts with an exception' do
+        allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_raise(JWT::DecodeError)
+
+        expect { make_request }.to raise_error(JWT::DecodeError)
+      end
+    end
+  end
+
+  shared_examples 'feature flag "build_service_proxy" is disabled' do
+    let(:user) { admin }
+
+    it 'returns 404' do
+      allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
+      stub_feature_flags(build_service_proxy: false)
+
+      make_request
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+
+  describe 'GET #proxy_websocket_authorize' do
+    let(:path) { :proxy_websocket_authorize }
+    let(:render_method) { :channel_websocket }
+    let(:expected_data) do
+      {
+        'Channel' => {
+          'Subprotocols' => ["terminal.gitlab.com"],
+          'Url' => 'wss://localhost/proxy/build/default_port/',
+          'Header' => {
+            'Authorization' => [nil]
+          },
+          'MaxSessionTime' => nil,
+          'CAPem' => nil
+        }
+      }.to_json
+    end
+
+    it_behaves_like 'proxy access rights'
+    it_behaves_like 'when pipeline is not from a webide source'
+    it_behaves_like 'validates workhorse signature'
+    it_behaves_like 'feature flag "build_service_proxy" is disabled'
+
+    it 'converts the url scheme into wss' do
+      allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
+
+      expect(job.runner_session_url).to start_with('https://')
+      expect(Gitlab::Workhorse).to receive(:channel_websocket).with(a_hash_including(url: "wss://localhost/proxy/build/default_port/"))
+
+      make_request
+    end
+  end
+
+  def make_request
+    params = {
+      namespace_id: project.namespace.to_param,
+      project_id: project
+    }
+
+    get path, params: params.merge(extra_params)
+  end
+end

ee/spec/models/ci/build_runner_session_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::BuildRunnerSession, model: true do
+  let!(:build) { create(:ci_build, :with_runner_session) }
+
+  subject { build.runner_session }
+
+  describe '#service_specification' do
+    let(:service) { 'foo'}
+    let(:port) { 80 }
+    let(:path) { 'path' }
+    let(:subprotocols) { nil }
+    let(:specification) { subject.service_specification(service: service, port: port, path: path, subprotocols: subprotocols) }
+
+    it 'returns service proxy url' do
+      expect(specification[:url]).to eq "https://localhost/proxy/#{service}/#{port}/#{path}"
+    end
+
+    it 'returns default service proxy websocket subprotocol' do
+      expect(specification[:subprotocols]).to eq %w[terminal.gitlab.com]
+    end
+
+    it 'returns empty hash if no url' do
+      subject.url = ''
+
+      expect(specification).to be_empty
+    end
+
+    context 'when port is not present' do
+      let(:port) { nil }
+
+      it 'uses the default port name' do
+        expect(specification[:url]).to eq "https://localhost/proxy/#{service}/default_port/#{path}"
+      end
+    end
+
+    context 'when the service is not present' do
+      let(:service) { '' }
+
+      it 'uses the service name "build" as default' do
+        expect(specification[:url]).to eq "https://localhost/proxy/build/#{port}/#{path}"
+      end
+    end
+
+    context 'when url is present' do
+      it 'returns ca_pem nil if empty certificate' do
+        subject.certificate = ''
+
+        expect(specification[:ca_pem]).to be_nil
+      end
+
+      it 'adds Authorization header if authorization is present' do
+        subject.authorization = 'foobar'
+
+        expect(specification[:headers]).to include(Authorization: ['foobar'])
+      end
+    end
+
+    context 'when subprotocol is present' do
+      let(:subprotocols) { 'foobar' }
+
+      it 'returns the new subprotocol' do
+        expect(specification[:subprotocols]).to eq [subprotocols]
+      end
+    end
+  end
+end

ee/spec/models/web_ide_terminal_spec.rb

@@ -22,4 +22,8 @@ describe WebIdeTerminal do
   it 'returns the terminal_path of the build' do
     expect(subject.terminal_path).to end_with("/jobs/#{build.id}/terminal.ws")
   end
+
+  it 'returns the proxy_websocket_path of the build' do
+    expect(subject.proxy_websocket_path).to end_with("/jobs/#{build.id}/proxy.ws")
+  end
 end

ee/spec/policies/ci/build_policy_spec.rb

@@ -34,7 +34,7 @@ describe Ci::BuildPolicy do
   end
 
   describe 'manage a web ide terminal' do
-    let(:build_permissions) { %i[read_web_ide_terminal create_build_terminal update_web_ide_terminal] }
+    let(:build_permissions) { %i[read_web_ide_terminal create_build_terminal update_web_ide_terminal create_build_service_proxy] }
     set(:maintainer) { create(:user) }
     let(:owner) { create(:owner) }
     let(:admin) { create(:admin) }
@@ -79,7 +79,7 @@ describe Ci::BuildPolicy do
         context 'when build is not from a webide pipeline' do
           let(:pipeline) { create(:ci_empty_pipeline, project: project, source: :chat) }
 
-          it { expect_disallowed(:read_web_ide_terminal, :update_web_ide_terminal) }
+          it { expect_disallowed(:read_web_ide_terminal, :update_web_ide_terminal, :create_build_service_proxy) }
         end
 
         context 'when build has no runner terminal' do
@@ -88,7 +88,15 @@ describe Ci::BuildPolicy do
           end
 
           it { expect_allowed(:read_web_ide_terminal, :update_web_ide_terminal) }
-          it { expect_disallowed(:create_build_terminal) }
+          it { expect_disallowed(:create_build_terminal, :create_build_service_proxy) }
+        end
+
+        context 'feature flag "build_service_proxy" is disabled' do
+          before do
+            stub_feature_flags(build_service_proxy: false)
+          end
+
+          it { expect_disallowed(:create_build_service_proxy) }
         end
       end
 

ee/spec/serializers/web_ide_terminal_entity_spec.rb

@@ -14,4 +14,20 @@ describe WebIdeTerminalEntity do
   it { is_expected.to have_key(:cancel_path) }
   it { is_expected.to have_key(:retry_path) }
   it { is_expected.to have_key(:terminal_path) }
+
+  context 'when feature flag build_service_proxy is enabled' do
+    before do
+      stub_feature_flags(build_service_proxy: true)
+    end
+
+    it { is_expected.to have_key(:proxy_websocket_path) }
+  end
+
+  context 'when feature flag build_service_proxy is disabled' do
+    before do
+      stub_feature_flags(build_service_proxy: false)
+    end
+
+    it { is_expected.not_to have_key(:proxy_websocket_path) }
+  end
 end