Merge branch 'ps-introduce-web-ide-oauth' into 'master'

Add web_ide_oauth feature flag

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138015



Merged-by: Terri Chu <tchu@gitlab.com>
Approved-by: Laura Callahan <lmeckley@gitlab.com>
Approved-by: Juliet Wanjohi <jwanjohi@gitlab.com>
Approved-by: Terri Chu <tchu@gitlab.com>
Reviewed-by: Terri Chu <tchu@gitlab.com>
Reviewed-by: Laura Callahan <lmeckley@gitlab.com>
Co-authored-by: Paul Slaughter <pslaughter@gitlab.com>

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,

app/assets/javascripts/ide/lib/gitlab_web_ide/get_oauth_config.js

+export const getOAuthConfig = ({ clientId, callbackUrl }) => {
+  if (!clientId) {
+    return undefined;
+  }
+
+  return {
+    type: 'oauth',
+    clientId,
+    callbackUrl,
+    protectRefreshToken: true,
+  };
+};

app/assets/javascripts/ide/lib/gitlab_web_ide/index.js

 export * from './get_base_config';
+export * from './get_oauth_config';
+export * from './handle_tracking_event';
 export * from './setup_root_element';

app/assets/javascripts/ide/mount_oauth_callback.js

+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),
+  });
+};

app/assets/javascripts/pages/ide/oauth_redirect/index.js

+import { mountOAuthCallback } from '~/ide/mount_oauth_callback';
+
+mountOAuthCallback();

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)

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:)

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

app/views/ide/oauth_redirect.html.haml

+- page_title _("IDE")
+
+= render partial: 'shared/ide_root', locals: { data: new_ide_oauth_data, loading_text: _('Authenticating...') }

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

config/feature_flags/development/web_ide_oauth.yml

+---
+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

config/routes.rb

@@ -131,6 +131,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|

lib/gitlab/web_ide/default_oauth_application.rb

+# 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

locale/gitlab.pot

@@ -7130,6 +7130,9 @@ msgstr ""
 msgid "Authenticated web requests"
 msgstr ""
+msgid "Authenticating..."
+msgstr ""
+
 msgid "Authentication"
 msgstr ""

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,
+        }),
+      );
+    });
+  });
 });

spec/frontend/ide/lib/gitlab_web_ide/get_oauth_config_spec.js

+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,
+    });
+  });
+});

spec/frontend/ide/mount_oauth_callback_spec.js

+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,
+    });
+  });
+});

spec/lib/gitlab/web_ide/default_oauth_application_spec.rb

+# 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

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