diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 2a4933e7bc2fcf2528b74b91de595dcf5543cf2e..4f00a484a5743eb9cdd163ba5efed56d40c7d694 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -208,3 +208,5 @@ def raw_trace_content_disposition(raw_data) 'attachment' end end + +Projects::JobsController.prepend(EE::Projects::JobsController) diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb index 997bf2980255517d4389788c822502204acb48bf..28f84ee1aa2847fbac7b63ddacbbb174b995af5b 100644 --- a/app/models/ci/build_runner_session.rb +++ b/app/models/ci/build_runner_session.rb @@ -37,3 +37,5 @@ def channel_specification(url, subprotocol) end end end + +Ci::BuildRunnerSession.prepend(EE::Ci::BuildRunnerSession) diff --git a/ee/app/controllers/ee/projects/jobs_controller.rb b/ee/app/controllers/ee/projects/jobs_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..03ee2752c1d231dc838b56ad525e26979aa6114e --- /dev/null +++ b/ee/app/controllers/ee/projects/jobs_controller.rb @@ -0,0 +1,52 @@ +# 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 diff --git a/ee/app/models/ee/ci/build.rb b/ee/app/models/ee/ci/build.rb index 5af96dc6f8e4667f1270282a0cfc9381a7c7274e..e699a132410234aa6cbba103c98aed20abaf7699 100644 --- a/ee/app/models/ee/ci/build.rb +++ b/ee/app/models/ee/ci/build.rb @@ -18,6 +18,7 @@ module Build 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, diff --git a/ee/app/models/ee/ci/build_runner_session.rb b/ee/app/models/ee/ci/build_runner_session.rb new file mode 100644 index 0000000000000000000000000000000000000000..549bb9e7c6c53c72a400121e3b9bbd588f6821ac --- /dev/null +++ b/ee/app/models/ee/ci/build_runner_session.rb @@ -0,0 +1,23 @@ +# 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 diff --git a/ee/app/models/web_ide_terminal.rb b/ee/app/models/web_ide_terminal.rb index a110abac3f1e4f3c3f83714ae9276d87c73c6a74..181d9f045face93266d9a40cde78cf429fa44152 100644 --- a/ee/app/models/web_ide_terminal.rb +++ b/ee/app/models/web_ide_terminal.rb @@ -28,14 +28,20 @@ def terminal_path 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 diff --git a/ee/app/policies/ee/ci/build_policy.rb b/ee/app/policies/ee/ci/build_policy.rb index 2bb1e17b1d8a6900b8655493c098817e74d1594a..08de6357d6e455a0fc5759bfb150af1d5c4fab87 100644 --- a/ee/app/policies/ee/ci/build_policy.rb +++ b/ee/app/policies/ee/ci/build_policy.rb @@ -26,6 +26,11 @@ module BuildPolicy 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 diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 142eac0c378f68c86b50e6c6b82b17a4381d4dad..e2b29fd245f0641b97d2f187d793fb4c7939bd62 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -192,6 +192,10 @@ module ProjectPolicy @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 ProjectPolicy prevent :create_design prevent :destroy_design end + + rule { build_service_proxy_enabled }.enable :build_service_proxy_enabled end end end diff --git a/ee/app/serializers/web_ide_terminal_entity.rb b/ee/app/serializers/web_ide_terminal_entity.rb index e8ec3a7feda0f498d69a18ccff384c45dcea8eec..617678231f7d985353cc4d931d7826ce2f7456a6 100644 --- a/ee/app/serializers/web_ide_terminal_entity.rb +++ b/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 diff --git a/ee/changelogs/unreleased/fj-5276-mirror-webide-changes.yml b/ee/changelogs/unreleased/fj-5276-mirror-webide-changes.yml new file mode 100644 index 0000000000000000000000000000000000000000..f7daa63f39344de3a85249a72375190268bd4573 --- /dev/null +++ b/ee/changelogs/unreleased/fj-5276-mirror-webide-changes.yml @@ -0,0 +1,5 @@ +--- +title: Proxy websocket requests to build services +merge_request: 9723 +author: +type: added diff --git a/ee/config/routes/project.rb b/ee/config/routes/project.rb index 4b67cb4af66ee784a4b25f3f404ef2a0b387ac13..3b50c04d6b08b8a368188d7118ca9a9a2ec31d2b 100644 --- a/ee/config/routes/project.rb +++ b/ee/config/routes/project.rb @@ -47,6 +47,13 @@ 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 diff --git a/ee/spec/controllers/ee/projects/jobs_controller_spec.rb b/ee/spec/controllers/ee/projects/jobs_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8095f1de4a7c1ac935b5bea72c9bd645aa879cd4 --- /dev/null +++ b/ee/spec/controllers/ee/projects/jobs_controller_spec.rb @@ -0,0 +1,200 @@ +# 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 diff --git a/ee/spec/models/ci/build_runner_session_spec.rb b/ee/spec/models/ci/build_runner_session_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2fc3155fefdaa23401cfb43e27085a01d4434d57 --- /dev/null +++ b/ee/spec/models/ci/build_runner_session_spec.rb @@ -0,0 +1,69 @@ +# 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 diff --git a/ee/spec/models/web_ide_terminal_spec.rb b/ee/spec/models/web_ide_terminal_spec.rb index 425218e56b765e161733457e6124d5b0199d3f6f..7d5c666aea782190ffd9a8f370f5d5f03f8526b8 100644 --- a/ee/spec/models/web_ide_terminal_spec.rb +++ b/ee/spec/models/web_ide_terminal_spec.rb @@ -22,4 +22,8 @@ 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 diff --git a/ee/spec/policies/ci/build_policy_spec.rb b/ee/spec/policies/ci/build_policy_spec.rb index cb1f576dfa789eeb564c5359d0063e43c873d4b8..2fc56e2e221dc70bf514198fdc63719d19b69f6d 100644 --- a/ee/spec/policies/ci/build_policy_spec.rb +++ b/ee/spec/policies/ci/build_policy_spec.rb @@ -34,7 +34,7 @@ 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 @@ 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 @@ 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 diff --git a/ee/spec/serializers/web_ide_terminal_entity_spec.rb b/ee/spec/serializers/web_ide_terminal_entity_spec.rb index 4bbe9f0e5f4573c89539f390b213beb180890c50..30651ddecf49f69f5626a02d4613dcfbea8b808e 100644 --- a/ee/spec/serializers/web_ide_terminal_entity_spec.rb +++ b/ee/spec/serializers/web_ide_terminal_entity_spec.rb @@ -14,4 +14,20 @@ 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