diff --git a/app/assets/javascripts/ide/init_gitlab_web_ide.js b/app/assets/javascripts/ide/init_gitlab_web_ide.js index 868830c953a5e371d560ed51be0f90be75bcd5a7..f5fb4c8be2fee96ff4a8edc507c061078e6278bd 100644 --- a/app/assets/javascripts/ide/init_gitlab_web_ide.js +++ b/app/assets/javascripts/ide/init_gitlab_web_ide.js @@ -6,10 +6,13 @@ import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_action'; import { createAndSubmitForm } from '~/lib/utils/create_and_submit_form'; import csrf from '~/lib/utils/csrf'; import Tracking from '~/tracking'; -import { getBaseConfig } from './lib/gitlab_web_ide/get_base_config'; -import { setupRootElement } from './lib/gitlab_web_ide/setup_root_element'; +import { + getBaseConfig, + getOAuthConfig, + setupRootElement, + handleTracking, +} from './lib/gitlab_web_ide'; import { GITLAB_WEB_IDE_FEEDBACK_ISSUE } from './constants'; -import { handleTracking } from './lib/gitlab_web_ide/handle_tracking_event'; const buildRemoteIdeURL = (ideRemotePath, remoteHost, remotePathArg) => { const remotePath = cleanLeadingSeparator(remotePathArg); @@ -51,15 +54,21 @@ export const initGitlabWebIDE = async (el) => { : null; const forkInfo = forkInfoJSON ? JSON.parse(forkInfoJSON) : null; + const oauthConfig = getOAuthConfig(el.dataset); + const httpHeaders = oauthConfig + ? undefined + : // Use same headers as defined in axios_utils (not needed in oauth) + { + [csrf.headerKey]: csrf.token, + 'X-Requested-With': 'XMLHttpRequest', + }; + // See ClientOnlyConfig https://gitlab.com/gitlab-org/gitlab-web-ide/-/blob/main/packages/web-ide-types/src/config.ts#L17 start(rootEl, { ...getBaseConfig(), nonce, - // Use same headers as defined in axios_utils - httpHeaders: { - [csrf.headerKey]: csrf.token, - 'X-Requested-With': 'XMLHttpRequest', - }, + httpHeaders, + auth: oauthConfig, projectPath, ref, filePath, diff --git a/app/assets/javascripts/ide/lib/gitlab_web_ide/get_oauth_config.js b/app/assets/javascripts/ide/lib/gitlab_web_ide/get_oauth_config.js new file mode 100644 index 0000000000000000000000000000000000000000..5493a9ba7c79bd58f4057ef71feb2a755ace5e3b --- /dev/null +++ b/app/assets/javascripts/ide/lib/gitlab_web_ide/get_oauth_config.js @@ -0,0 +1,12 @@ +export const getOAuthConfig = ({ clientId, callbackUrl }) => { + if (!clientId) { + return undefined; + } + + return { + type: 'oauth', + clientId, + callbackUrl, + protectRefreshToken: true, + }; +}; diff --git a/app/assets/javascripts/ide/lib/gitlab_web_ide/index.js b/app/assets/javascripts/ide/lib/gitlab_web_ide/index.js index 8311e11672efbf9fbb6e646234ae2b520550ea33..87e0002c8c8e89bca8388c27ee22289a0230b1cd 100644 --- a/app/assets/javascripts/ide/lib/gitlab_web_ide/index.js +++ b/app/assets/javascripts/ide/lib/gitlab_web_ide/index.js @@ -1,2 +1,4 @@ export * from './get_base_config'; +export * from './get_oauth_config'; +export * from './handle_tracking_event'; export * from './setup_root_element'; diff --git a/app/assets/javascripts/ide/mount_oauth_callback.js b/app/assets/javascripts/ide/mount_oauth_callback.js new file mode 100644 index 0000000000000000000000000000000000000000..79fffb24f8eff264cdb8db6d0269aca0f1ac0fe2 --- /dev/null +++ b/app/assets/javascripts/ide/mount_oauth_callback.js @@ -0,0 +1,12 @@ +import { oauthCallback } from '@gitlab/web-ide'; +import { getBaseConfig, getOAuthConfig } from './lib/gitlab_web_ide'; + +export const mountOAuthCallback = () => { + const el = document.getElementById('ide'); + + return oauthCallback({ + ...getBaseConfig(), + username: gon.current_username, + auth: getOAuthConfig(el.dataset), + }); +}; diff --git a/app/assets/javascripts/pages/ide/index.js b/app/assets/javascripts/pages/ide/index/index.js similarity index 100% rename from app/assets/javascripts/pages/ide/index.js rename to app/assets/javascripts/pages/ide/index/index.js diff --git a/app/assets/javascripts/pages/ide/oauth_redirect/index.js b/app/assets/javascripts/pages/ide/oauth_redirect/index.js new file mode 100644 index 0000000000000000000000000000000000000000..ee9233fab38cae18d0c70a28de30b310b1f3a7c8 --- /dev/null +++ b/app/assets/javascripts/pages/ide/oauth_redirect/index.js @@ -0,0 +1,3 @@ +import { mountOAuthCallback } from '~/ide/mount_oauth_callback'; + +mountOAuthCallback(); diff --git a/app/controllers/ide_controller.rb b/app/controllers/ide_controller.rb index d9566121dcd72a942ab8c17d788d43d85acebf0f..4a4d41f3e6fe735d1cb451a07d2c802fe6f1c3dc 100644 --- a/app/controllers/ide_controller.rb +++ b/app/controllers/ide_controller.rb @@ -5,7 +5,8 @@ class IdeController < ApplicationController include StaticObjectExternalStorageCSP include Gitlab::Utils::StrongMemoize - before_action :authorize_read_project! + before_action :authorize_read_project!, only: [:index] + before_action :ensure_web_ide_oauth_application!, only: [:index] before_action do push_frontend_feature_flag(:build_service_proxy) @@ -27,12 +28,28 @@ def index render layout: helpers.use_new_web_ide? ? 'fullscreen' : 'application' end + def oauth_redirect + return render_404 unless ::Gitlab::WebIde::DefaultOauthApplication.feature_enabled?(current_user) + # TODO - It's **possible** we end up here and no oauth application has been set up. + # We need to have better handling of these edge cases. Here's a follow-up issue: + # https://gitlab.com/gitlab-org/gitlab/-/issues/433322 + return render_404 unless ::Gitlab::WebIde::DefaultOauthApplication.oauth_application + + render layout: 'fullscreen', locals: { minimal: true } + end + private def authorize_read_project! render_404 unless can?(current_user, :read_project, project) end + def ensure_web_ide_oauth_application! + return unless ::Gitlab::WebIde::DefaultOauthApplication.feature_enabled?(current_user) + + ::Gitlab::WebIde::DefaultOauthApplication.ensure_oauth_application! + end + def fork_info(project, branch) return if can?(current_user, :push_code, project) diff --git a/app/helpers/ide_helper.rb b/app/helpers/ide_helper.rb index f2d393f1f77d215e9ed2e1d433b65c62c07a3662..2ec11b8a9ed9e5ab0efe945e68e2295637c53de1 100644 --- a/app/helpers/ide_helper.rb +++ b/app/helpers/ide_helper.rb @@ -52,6 +52,19 @@ def new_ide_code_suggestions_data {} end + def new_ide_oauth_data + return {} unless ::Gitlab::WebIde::DefaultOauthApplication.feature_enabled?(current_user) + return {} unless ::Gitlab::WebIde::DefaultOauthApplication.oauth_application + + client_id = ::Gitlab::WebIde::DefaultOauthApplication.oauth_application.uid + callback_url = ::Gitlab::WebIde::DefaultOauthApplication.oauth_callback_url + + { + 'client-id' => client_id, + 'callback-url' => callback_url + } + end + def new_ide_data(project:) { 'project-path' => project&.path_with_namespace, @@ -59,7 +72,7 @@ def new_ide_data(project:) # We will replace these placeholders in the FE 'ide-remote-path' => ide_remote_path(remote_host: ':remote_host', remote_path: ':remote_path'), 'editor-font' => new_ide_fonts.to_json - }.merge(new_ide_code_suggestions_data) + }.merge(new_ide_code_suggestions_data).merge(new_ide_oauth_data) end def legacy_ide_data(project:) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a33d7e33769ec00c26213dbe77384e19d8163fb3..0b816517deac9ed379e0c9b124f64f6b0e44757f 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -42,6 +42,7 @@ class ApplicationSetting < MainClusterwide::ApplicationRecord add_authentication_token_field :error_tracking_access_token, encrypted: :required belongs_to :push_rule + belongs_to :web_ide_oauth_application, class_name: 'Doorkeeper::Application' alias_attribute :housekeeping_optimize_repository_period, :housekeeping_incremental_repack_period diff --git a/app/views/ide/oauth_redirect.html.haml b/app/views/ide/oauth_redirect.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..e0ce9f9768c2164bca6c71185ed9bd868696855e --- /dev/null +++ b/app/views/ide/oauth_redirect.html.haml @@ -0,0 +1,3 @@ +- page_title _("IDE") + += render partial: 'shared/ide_root', locals: { data: new_ide_oauth_data, loading_text: _('Authenticating...') } diff --git a/app/views/shared/_ide_root.html.haml b/app/views/shared/_ide_root.html.haml index db3e76e188c4dfdf593217cc99097791bebfb078..173b081d69328e4c435191a92866b4fa0229e6ea 100644 --- a/app/views/shared/_ide_root.html.haml +++ b/app/views/shared/_ide_root.html.haml @@ -5,6 +5,6 @@ -# 100vh because of the presence of the bottom bar #ide.gl-h-full{ data: data } - .web-ide-loader.gl-display-flex.gl-justify-content-center.gl-align-items-center.gl-flex-direction-column.gl-h-full.gl-mr-auto.gl-ml-auto + .web-ide-loader.gl-display-flex.gl-justify-content-center.gl-align-items-center.gl-flex-direction-column.gl-h-full.gl-mx-auto = brand_header_logo %h3.clblack.gl-mt-6= loading_text diff --git a/config/feature_flags/development/web_ide_oauth.yml b/config/feature_flags/development/web_ide_oauth.yml new file mode 100644 index 0000000000000000000000000000000000000000..fc3132d01b400168f312cce9d759c38c43aa46ef --- /dev/null +++ b/config/feature_flags/development/web_ide_oauth.yml @@ -0,0 +1,8 @@ +--- +name: web_ide_oauth +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138015 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/433324 +milestone: '16.7' +type: development +group: group::ide +default_enabled: false diff --git a/config/routes.rb b/config/routes.rb index fc6a7a377151b92e9f504d5321895225b18cb251..018c9633d94fbf0719c47c96ed6843b60a4a3968 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -130,6 +130,7 @@ scope :ide, as: :ide, format: false do get '/', to: 'ide#index' get '/project', to: 'ide#index' + get '/oauth_redirect', to: 'ide#oauth_redirect' scope path: 'project/:project_id', as: :project, constraints: { project_id: Gitlab::PathRegex.full_namespace_route_regex } do %w[edit tree blob].each do |action| diff --git a/lib/gitlab/web_ide/default_oauth_application.rb b/lib/gitlab/web_ide/default_oauth_application.rb new file mode 100644 index 0000000000000000000000000000000000000000..01b7637c1c081de2f291a294ba6a122893a1005b --- /dev/null +++ b/lib/gitlab/web_ide/default_oauth_application.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Gitlab + module WebIde + module DefaultOauthApplication + class << self + def feature_enabled?(current_user) + Feature.enabled?(:vscode_web_ide, current_user) && Feature.enabled?(:web_ide_oauth, current_user) + end + + def oauth_application + application_settings.web_ide_oauth_application + end + + def oauth_callback_url + Gitlab::Routing.url_helpers.ide_oauth_redirect_url + end + + def ensure_oauth_application! + return if oauth_application + + should_expire_cache = false + + application_settings.transaction do + # note: This should run very rarely and should be safe for us to do a lock + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132496#note_1587293087 + application_settings.lock! + + # note: `lock!`` breaks applicaiton_settings cache and will trigger another query. + # We need to double check here so that requests previously waiting on the lock can + # now just skip. + next if oauth_application + + application = Doorkeeper::Application.new( + name: 'GitLab Web IDE', + redirect_uri: oauth_callback_url, + scopes: ['api'], + trusted: true, + confidential: false) + application.save! + application_settings.update!(web_ide_oauth_application: application) + should_expire_cache = true + end + + # note: This needs to happen outside the transaction, but only if we actually changed something + ::Gitlab::CurrentSettings.expire_current_application_settings if should_expire_cache + end + + private + + def application_settings + ::Gitlab::CurrentSettings.current_application_settings + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index af4930a341b82442c0a49892413459bb5c6b6b7d..acaf1ced98386919949eed184dac631290921cf3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7127,6 +7127,9 @@ msgstr "" msgid "Authenticated web requests" msgstr "" +msgid "Authenticating..." +msgstr "" + msgid "Authentication" msgstr "" diff --git a/spec/frontend/ide/init_gitlab_web_ide_spec.js b/spec/frontend/ide/init_gitlab_web_ide_spec.js index 6a5bedb0bbb3326d1dc6907651f49e09cae8271c..d7a16bec1c3ebbce3263c558bff25531e8cd0973 100644 --- a/spec/frontend/ide/init_gitlab_web_ide_spec.js +++ b/spec/frontend/ide/init_gitlab_web_ide_spec.js @@ -40,6 +40,9 @@ const TEST_EDITOR_FONT_SRC_URL = 'http://gitlab.test/assets/gitlab-mono/GitLabMo const TEST_EDITOR_FONT_FORMAT = 'woff2'; const TEST_EDITOR_FONT_FAMILY = 'GitLab Mono'; +const TEST_OAUTH_CLIENT_ID = 'oauth-client-id-123abc'; +const TEST_OAUTH_CALLBACK_URL = 'https://example.com/oauth_callback'; + describe('ide/init_gitlab_web_ide', () => { let resolveConfirm; @@ -231,4 +234,29 @@ describe('ide/init_gitlab_web_ide', () => { ); }); }); + + describe('when oauth info is in dataset', () => { + beforeEach(() => { + findRootElement().dataset.clientId = TEST_OAUTH_CLIENT_ID; + findRootElement().dataset.callbackUrl = TEST_OAUTH_CALLBACK_URL; + + createSubject(); + }); + + it('calls start with element', () => { + expect(start).toHaveBeenCalledTimes(1); + expect(start).toHaveBeenCalledWith( + findRootElement(), + expect.objectContaining({ + auth: { + type: 'oauth', + clientId: TEST_OAUTH_CLIENT_ID, + callbackUrl: TEST_OAUTH_CALLBACK_URL, + protectRefreshToken: true, + }, + httpHeaders: undefined, + }), + ); + }); + }); }); diff --git a/spec/frontend/ide/lib/gitlab_web_ide/get_oauth_config_spec.js b/spec/frontend/ide/lib/gitlab_web_ide/get_oauth_config_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..3431068937f1ebaf0c09c2d7bfdfedf043fc6253 --- /dev/null +++ b/spec/frontend/ide/lib/gitlab_web_ide/get_oauth_config_spec.js @@ -0,0 +1,16 @@ +import { getOAuthConfig } from '~/ide/lib/gitlab_web_ide/get_oauth_config'; + +describe('~/ide/lib/gitlab_web_ide/get_oauth_config', () => { + it('returns undefined if no clientId found', () => { + expect(getOAuthConfig({})).toBeUndefined(); + }); + + it('returns auth config from dataset', () => { + expect(getOAuthConfig({ clientId: 'test-clientId', callbackUrl: 'test-callbackUrl' })).toEqual({ + type: 'oauth', + clientId: 'test-clientId', + callbackUrl: 'test-callbackUrl', + protectRefreshToken: true, + }); + }); +}); diff --git a/spec/frontend/ide/mount_oauth_callback_spec.js b/spec/frontend/ide/mount_oauth_callback_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..6ac0b4e46152377bf24bfeb4271db0548915ab8f --- /dev/null +++ b/spec/frontend/ide/mount_oauth_callback_spec.js @@ -0,0 +1,53 @@ +import { oauthCallback } from '@gitlab/web-ide'; +import { TEST_HOST } from 'helpers/test_constants'; +import { mountOAuthCallback } from '~/ide/mount_oauth_callback'; + +jest.mock('@gitlab/web-ide'); + +const TEST_USERNAME = 'gandalf.the.grey'; +const TEST_GITLAB_WEB_IDE_PUBLIC_PATH = 'test/webpack/assets/gitlab-web-ide/public/path'; + +const TEST_OAUTH_CLIENT_ID = 'oauth-client-id-123abc'; +const TEST_OAUTH_CALLBACK_URL = 'https://example.com/oauth_callback'; + +describe('~/ide/mount_oauth_callback', () => { + const createRootElement = () => { + const el = document.createElement('div'); + + el.id = 'ide'; + el.dataset.clientId = TEST_OAUTH_CLIENT_ID; + el.dataset.callbackUrl = TEST_OAUTH_CALLBACK_URL; + + document.body.append(el); + }; + + beforeEach(() => { + gon.current_username = TEST_USERNAME; + process.env.GITLAB_WEB_IDE_PUBLIC_PATH = TEST_GITLAB_WEB_IDE_PUBLIC_PATH; + + createRootElement(); + }); + + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('calls oauthCallback', () => { + expect(oauthCallback).not.toHaveBeenCalled(); + + mountOAuthCallback(); + + expect(oauthCallback).toHaveBeenCalledTimes(1); + expect(oauthCallback).toHaveBeenCalledWith({ + auth: { + type: 'oauth', + callbackUrl: TEST_OAUTH_CALLBACK_URL, + clientId: TEST_OAUTH_CLIENT_ID, + protectRefreshToken: true, + }, + gitlabUrl: TEST_HOST, + baseUrl: `${TEST_HOST}/${TEST_GITLAB_WEB_IDE_PUBLIC_PATH}`, + username: TEST_USERNAME, + }); + }); +}); diff --git a/spec/lib/gitlab/web_ide/default_oauth_application_spec.rb b/spec/lib/gitlab/web_ide/default_oauth_application_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9bfdc799aec2d4f79788e0979a63680cd3aef233 --- /dev/null +++ b/spec/lib/gitlab/web_ide/default_oauth_application_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::WebIde::DefaultOauthApplication, feature_category: :web_ide do + let_it_be(:current_user) { create(:user) } + let_it_be(:oauth_application) { create(:oauth_application, owner: nil) } + + describe '#feature_enabled?' do + where(:vscode_web_ide, :web_ide_oauth, :expectation) do + [ + [ref(:current_user), false, false], + [false, ref(:current_user), false], + [ref(:current_user), ref(:current_user), true] + ] + end + + with_them do + it 'returns the expected value' do + stub_feature_flags(vscode_web_ide: vscode_web_ide, web_ide_oauth: web_ide_oauth) + + expect(described_class.feature_enabled?(current_user)).to be(expectation) + end + end + end + + describe '#oauth_application' do + it 'returns web_ide_oauth_application from application_settings' do + expect(described_class.oauth_application).to be_nil + + stub_application_setting({ web_ide_oauth_application: oauth_application }) + + expect(described_class.oauth_application).to be(oauth_application) + end + end + + describe '#oauth_callback_url' do + it 'returns route URL for oauth callback' do + expect(described_class.oauth_callback_url).to eq(Gitlab::Routing.url_helpers.ide_oauth_redirect_url) + end + end + + describe '#ensure_oauth_application!' do + it 'if web_ide_oauth_application already exists, does nothing' do + expect(application_settings).not_to receive(:lock!) + expect(::Doorkeeper::Application).not_to receive(:new) + + stub_application_setting({ web_ide_oauth_application: oauth_application }) + + described_class.ensure_oauth_application! + end + + it 'if web_ide_oauth_application created while locked, does nothing' do + expect(application_settings).to receive(:lock!) do + stub_application_setting({ web_ide_oauth_application: oauth_application }) + end + expect(::Doorkeeper::Application).not_to receive(:new) + expect(::Gitlab::CurrentSettings).not_to receive(:expire_current_application_settings) + + described_class.ensure_oauth_application! + end + + it 'creates web_ide_oauth_application' do + expect(application_settings).to receive(:transaction).and_call_original + expect(::Doorkeeper::Application).to receive(:new).and_call_original + expect(::Gitlab::CurrentSettings).to receive(:expire_current_application_settings).and_call_original + + expect(application_settings.web_ide_oauth_application).to be_nil + + described_class.ensure_oauth_application! + + result = application_settings.web_ide_oauth_application + expect(result).not_to be_nil + expect(result).to have_attributes( + name: 'GitLab Web IDE', + redirect_uri: described_class.oauth_callback_url, + scopes: ['api'], + trusted: true, + confidential: false + ) + end + end + + def application_settings + ::Gitlab::CurrentSettings.current_application_settings + end +end diff --git a/spec/requests/ide_controller_spec.rb b/spec/requests/ide_controller_spec.rb index fae3d9533c50547a1f6491fd4e0b1d8a494692b3..20d890fadbf12a4f8d03373284ef5e65bd39c1c5 100644 --- a/spec/requests/ide_controller_spec.rb +++ b/spec/requests/ide_controller_spec.rb @@ -164,6 +164,14 @@ expect(response).to render_template('layouts/application') end + + it 'does not create oauth application' do + expect(Doorkeeper::Application).not_to receive(:new) + + subject + + expect(web_ide_oauth_application).to be_nil + end end describe 'vscode IDE' do @@ -177,6 +185,40 @@ expect(response).to render_template('layouts/fullscreen') end end + + describe 'with web_ide_oauth flag off' do + before do + stub_feature_flags(web_ide_oauth: false) + end + + it 'does not create oauth application' do + expect(Doorkeeper::Application).not_to receive(:new) + + subject + + expect(web_ide_oauth_application).to be_nil + end + end + + it 'ensures web_ide_oauth_application' do + expect(Doorkeeper::Application).to receive(:new).and_call_original + + subject + + expect(web_ide_oauth_application).not_to be_nil + expect(web_ide_oauth_application[:name]).to eq('GitLab Web IDE') + end + + it 'when web_ide_oauth_application already exists, does not create new one' do + existing_app = create(:oauth_application, owner_id: nil, owner_type: nil) + + stub_application_setting({ web_ide_oauth_application: existing_app }) + expect(Doorkeeper::Application).not_to receive(:new) + + subject + + expect(web_ide_oauth_application).to eq(existing_app) + end end describe 'content security policy' do @@ -199,4 +241,48 @@ end end end + + describe '#oauth_redirect', :aggregate_failures do + subject(:oauth_redirect) { get '/-/ide/oauth_redirect' } + + it 'with no web_ide_oauth_application, returns not_found' do + oauth_redirect + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'with web_ide_oauth_application set' do + before do + stub_application_setting({ + web_ide_oauth_application: create(:oauth_application, owner_id: nil, owner_type: nil) + }) + end + + it 'returns ok and renders view' do + oauth_redirect + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'with vscode_web_ide flag off, returns not_found' do + stub_feature_flags(vscode_web_ide: false) + + oauth_redirect + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'with web_ide_oauth flag off, returns not_found' do + stub_feature_flags(web_ide_oauth: false) + + oauth_redirect + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + def web_ide_oauth_application + ::Gitlab::CurrentSettings.current_application_settings.web_ide_oauth_application + end end