From e1592d9c05ee1c6f35850434a2115948b6425239 Mon Sep 17 00:00:00 2001 From: Cindy Halim <chalim@gitlab.com> Date: Wed, 17 Jul 2024 15:34:14 +0000 Subject: [PATCH] Web IDE OAuth Application callout Create callout component and add unit tests. --- .../components/oauth_application_callout.vue | 93 ++++++++++++++ .../reset_application_settings_modal.vue | 103 +++++++++++++++ .../ide/oauth_application_callout.js | 25 ++++ .../pages/admin/applications/index.js | 2 + .../admin/applications_controller.rb | 8 ++ ...eb_ide_oauth_application_callout.html.haml | 3 + app/views/admin/applications/edit.html.haml | 2 + config/routes.rb | 2 + lib/web_ide/default_oauth_application.rb | 25 ++-- locale/gitlab.pot | 33 +++++ .../admin/applications_controller_spec.rb | 25 ++++ .../oauth_application_callout_spec.js | 56 +++++++++ .../reset_application_settings_modal_spec.js | 118 ++++++++++++++++++ .../web_ide/default_oauth_application_spec.rb | 20 +++ 14 files changed, 508 insertions(+), 7 deletions(-) create mode 100644 app/assets/javascripts/ide/components/oauth_application_callout.vue create mode 100644 app/assets/javascripts/ide/components/reset_application_settings_modal.vue create mode 100644 app/assets/javascripts/ide/oauth_application_callout.js create mode 100644 app/views/admin/applications/_web_ide_oauth_application_callout.html.haml create mode 100644 spec/frontend/ide/components/oauth_application_callout_spec.js create mode 100644 spec/frontend/ide/components/reset_application_settings_modal_spec.js diff --git a/app/assets/javascripts/ide/components/oauth_application_callout.vue b/app/assets/javascripts/ide/components/oauth_application_callout.vue new file mode 100644 index 0000000000000..89805a434bfab --- /dev/null +++ b/app/assets/javascripts/ide/components/oauth_application_callout.vue @@ -0,0 +1,93 @@ +<script> +import { GlAlert, GlSprintf } from '@gitlab/ui'; + +import { s__ } from '~/locale'; +import ResetApplicationSettingsModal from './reset_application_settings_modal.vue'; + +export const I18N_WEB_IDE_OAUTH_APPLICATION_CALLOUT = { + alertTitle: s__( + 'IDE|Editing this application might affect the functionality of the Web IDE. Ensure the configuration meets the following conditions:', + ), + alertButtonText: s__('IDE|Restore to default'), + configurations: [ + s__( + 'IDE|The redirect URI path is %{codeBlockStart}%{pathFormat}%{codeBlockEnd}. An example of a valid redirect URI is %{codeBlockStart}%{example}%{codeBlockEnd}.', + ), + s__('IDE|The %{boldStart}Trusted%{boldEnd} checkbox is selected.'), + s__('IDE|The %{boldStart}Confidential%{boldEnd} checkbox is cleared.'), + s__('IDE|The %{boldStart}api%{boldEnd} scope is selected.'), + ], +}; + +export default { + name: 'WebIdeOAuthApplicationCallout', + components: { + GlAlert, + GlSprintf, + ResetApplicationSettingsModal, + }, + props: { + redirectUrlPath: { + type: String, + required: true, + }, + resetApplicationSettingsPath: { + type: String, + required: true, + }, + }, + data() { + return { + isModalVisible: false, + }; + }, + methods: { + displayModal() { + this.isModalVisible = true; + }, + hideModal() { + this.isModalVisible = false; + }, + getRedirectUrl() { + return new URL(this.redirectUrlPath, window.location.origin); + }, + }, + i18n: I18N_WEB_IDE_OAUTH_APPLICATION_CALLOUT, +}; +</script> +<template> + <div> + <reset-application-settings-modal + :visible="isModalVisible" + :reset-application-settings-path="resetApplicationSettingsPath" + @cancel="hideModal" + @close="hideModal" + /> + <gl-alert + variant="info" + class="gl-my-5" + :dismissible="false" + :primary-button-text="$options.i18n.alertButtonText" + @primaryAction="displayModal" + > + <p>{{ $options.i18n.alertTitle }}</p> + <ul class="gl-m-0"> + <li v-for="(message, index) in $options.i18n.configurations" :key="index"> + <gl-sprintf :message="message"> + <template #bold="{ content }"> + <strong>{{ content }}</strong> + </template> + <template #codeBlock="{ content }"> + <code>{{ + sprintf(content, { + pathFormat: redirectUrlPath, + example: `${getRedirectUrl()}`, + }) + }}</code> + </template> + </gl-sprintf> + </li> + </ul> + </gl-alert> + </div> +</template> diff --git a/app/assets/javascripts/ide/components/reset_application_settings_modal.vue b/app/assets/javascripts/ide/components/reset_application_settings_modal.vue new file mode 100644 index 0000000000000..45442f04068ce --- /dev/null +++ b/app/assets/javascripts/ide/components/reset_application_settings_modal.vue @@ -0,0 +1,103 @@ +<script> +import { GlAlert, GlModal } from '@gitlab/ui'; +import { s__ } from '~/locale'; +import axios from '~/lib/utils/axios_utils'; + +export const I18N_RESET_APPLICATION_SETTINGS_MODAL = { + errorMessage: s__( + 'IDE|An error occurred while restoring the application to default. Please try again.', + ), + title: s__('IDE|Restore application to default'), + body: s__( + 'IDE|Are you sure you want to restore this application to its original configuration? All your changes will be lost.', + ), + primaryButton: s__('IDE|Confirm'), + cancel: s__('IDE|Cancel'), +}; + +export default { + name: 'ResetApplicationSettingsModal', + components: { + GlAlert, + GlModal, + }, + props: { + visible: { + type: Boolean, + required: true, + }, + resetApplicationSettingsPath: { + type: String, + required: true, + }, + }, + data() { + return { + loading: false, + showErrorAlert: false, + }; + }, + computed: { + modalActionPrimary() { + return { + text: I18N_RESET_APPLICATION_SETTINGS_MODAL.primaryButton, + attributes: { + variant: 'danger', + id: 'confirm-restore-button', + loading: this.loading, + type: 'submit', + }, + }; + }, + modalActionSecondary() { + return { + text: I18N_RESET_APPLICATION_SETTINGS_MODAL.cancel, + attributes: { + loading: this.loading, + }, + }; + }, + }, + methods: { + handlePrimaryButtonClick(event) { + event.preventDefault(); + this.resetApplicationSetings(); + }, + async resetApplicationSetings() { + this.loading = true; + this.showErrorAlert = false; + try { + await axios.post(this.resetApplicationSettingsPath); + this.$refs.modal.hide(); + window.location.reload(); + } catch (e) { + this.showErrorAlert = true; + } + + this.loading = false; + }, + }, + i18n: I18N_RESET_APPLICATION_SETTINGS_MODAL, +}; +</script> +<template> + <gl-modal + ref="modal" + size="sm" + modal-id="reset-application-settings-modal" + :visible="visible" + :title="$options.i18n.title" + :action-primary="modalActionPrimary" + :action-secondary="modalActionSecondary" + v-bind="$attrs" + v-on="$listeners" + @primary="handlePrimaryButtonClick" + > + <div> + <gl-alert v-if="showErrorAlert" variant="danger" class="gl-mb-5" :dismissible="false"> + {{ $options.i18n.errorMessage }} + </gl-alert> + <p>{{ $options.i18n.body }}</p> + </div> + </gl-modal> +</template> diff --git a/app/assets/javascripts/ide/oauth_application_callout.js b/app/assets/javascripts/ide/oauth_application_callout.js new file mode 100644 index 0000000000000..a9d336c7ff2c7 --- /dev/null +++ b/app/assets/javascripts/ide/oauth_application_callout.js @@ -0,0 +1,25 @@ +import Vue from 'vue'; +import WebIdeOAuthApplicationCallout from './components/oauth_application_callout.vue'; + +export const initWebIdeOAuthApplicationCallout = () => { + const el = document.querySelector('#web_ide_oauth_application_callout'); + + if (!el) { + return null; + } + + const { redirectUrlPath, resetApplicationSettingsPath } = el.dataset; + + return new Vue({ + el, + name: 'WebIdeOAuthApplicationCallout', + render(h) { + return h(WebIdeOAuthApplicationCallout, { + props: { + redirectUrlPath, + resetApplicationSettingsPath, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/pages/admin/applications/index.js b/app/assets/javascripts/pages/admin/applications/index.js index df9e38431b06a..3d52ea1530a45 100644 --- a/app/assets/javascripts/pages/admin/applications/index.js +++ b/app/assets/javascripts/pages/admin/applications/index.js @@ -1,5 +1,7 @@ import initApplicationDeleteButtons from '~/admin/applications'; import { initOAuthApplicationSecret } from '~/oauth_application'; +import { initWebIdeOAuthApplicationCallout } from '~/ide/oauth_application_callout'; initApplicationDeleteButtons(); initOAuthApplicationSecret(); +initWebIdeOAuthApplicationCallout(); diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index d97fcc5df7483..c9419354c2324 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -58,6 +58,14 @@ def destroy redirect_to admin_applications_url, status: :found, notice: _('Application was successfully destroyed.') end + def reset_web_ide_oauth_application_settings + success = ::WebIde::DefaultOauthApplication.reset_oauth_application_settings + + return render json: {}, status: :internal_server_error unless success + + render json: {} + end + private def set_application diff --git a/app/views/admin/applications/_web_ide_oauth_application_callout.html.haml b/app/views/admin/applications/_web_ide_oauth_application_callout.html.haml new file mode 100644 index 0000000000000..aa72307a09014 --- /dev/null +++ b/app/views/admin/applications/_web_ide_oauth_application_callout.html.haml @@ -0,0 +1,3 @@ +- return unless web_ide_oauth_application_id == application.id + +#web_ide_oauth_application_callout{ :data => { :redirect_url_path => ide_oauth_redirect_path, :reset_application_settings_path => ide_reset_oauth_application_settings_path } } diff --git a/app/views/admin/applications/edit.html.haml b/app/views/admin/applications/edit.html.haml index 10a27fb906fd0..d0ecba3f977c8 100644 --- a/app/views/admin/applications/edit.html.haml +++ b/app/views/admin/applications/edit.html.haml @@ -2,6 +2,8 @@ - breadcrumb_title @application.name - page_title _("Edit"), @application.name, _("Applications") += render 'web_ide_oauth_application_callout', application: @application + %h1.page-title.gl-font-size-h-display = _('Edit application') - @url = admin_application_path(@application) diff --git a/config/routes.rb b/config/routes.rb index 868934605a9e2..c4eeaebc94a53 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -164,6 +164,8 @@ as: :remote, to: 'web_ide/remote_ide#index', constraints: { remote_host: %r{[^/?]+} } + + post '/reset_oauth_application_settings' => 'admin/applications#reset_web_ide_oauth_application_settings' end draw :operations diff --git a/lib/web_ide/default_oauth_application.rb b/lib/web_ide/default_oauth_application.rb index b9108914c9a96..d7b5a3a3b494b 100644 --- a/lib/web_ide/default_oauth_application.rb +++ b/lib/web_ide/default_oauth_application.rb @@ -25,6 +25,12 @@ def oauth_application_callback_urls URI.extract(oauth_application.redirect_uri, %w[http https]).uniq end + def reset_oauth_application_settings + return unless oauth_application + + oauth_application.update!(default_settings) + end + def ensure_oauth_application! return if oauth_application @@ -35,17 +41,12 @@ def ensure_oauth_application! # 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. + # note: `lock!`` breaks application_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 = Doorkeeper::Application.new(default_settings) application.save! application_settings.update!(web_ide_oauth_application: application) should_expire_cache = true @@ -60,6 +61,16 @@ def ensure_oauth_application! def application_settings ::Gitlab::CurrentSettings.current_application_settings end + + def default_settings + { + "name" => 'GitLab Web IDE', + "redirect_uri" => oauth_callback_url, + "scopes" => ['api'], + "trusted" => true, + "confidential" => false + }.freeze + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4d45fe2e2275c..a7ae06e3302a0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26516,6 +26516,15 @@ msgstr "" msgid "IDE" msgstr "" +msgid "IDE|An error occurred while restoring the application to default. Please try again." +msgstr "" + +msgid "IDE|Are you sure you want to restore this application to its original configuration? All your changes will be lost." +msgstr "" + +msgid "IDE|Cancel" +msgstr "" + msgid "IDE|Cannot open Web IDE" msgstr "" @@ -26525,12 +26534,18 @@ msgstr "" msgid "IDE|Commit to %{branchName} branch" msgstr "" +msgid "IDE|Confirm" +msgstr "" + msgid "IDE|Contact your administrator or try to open the Web IDE again with another domain." msgstr "" msgid "IDE|Edit" msgstr "" +msgid "IDE|Editing this application might affect the functionality of the Web IDE. Ensure the configuration meets the following conditions:" +msgstr "" + msgid "IDE|Error reloading page" msgstr "" @@ -26549,6 +26564,12 @@ msgstr "" msgid "IDE|Reopen with other domain" msgstr "" +msgid "IDE|Restore application to default" +msgstr "" + +msgid "IDE|Restore to default" +msgstr "" + msgid "IDE|Review" msgstr "" @@ -26558,9 +26579,21 @@ msgstr "" msgid "IDE|Successful commit" msgstr "" +msgid "IDE|The %{boldStart}Confidential%{boldEnd} checkbox is cleared." +msgstr "" + +msgid "IDE|The %{boldStart}Trusted%{boldEnd} checkbox is selected." +msgstr "" + +msgid "IDE|The %{boldStart}api%{boldEnd} scope is selected." +msgstr "" + msgid "IDE|The URL you're using to access the Web IDE and the configured OAuth callback URL do not match. This issue often occurs when you're using a proxy." msgstr "" +msgid "IDE|The redirect URI path is %{codeBlockStart}%{pathFormat}%{codeBlockEnd}. An example of a valid redirect URI is %{codeBlockStart}%{example}%{codeBlockEnd}." +msgstr "" + msgid "IDE|This option is disabled because you are not allowed to create merge requests in this project." msgstr "" diff --git a/spec/controllers/admin/applications_controller_spec.rb b/spec/controllers/admin/applications_controller_spec.rb index 1feda0ed36ff1..fa15ba607f7da 100644 --- a/spec/controllers/admin/applications_controller_spec.rb +++ b/spec/controllers/admin/applications_controller_spec.rb @@ -152,4 +152,29 @@ end end end + + describe "#reset_oauth_application_settings" do + subject(:reset_oauth_application_settings) { post :reset_web_ide_oauth_application_settings } + + before do + stub_feature_flags(web_ide_oauth: true) + end + + it 'returns 500 if no oauth application exists' do + stub_application_setting(web_ide_oauth_application: nil) + reset_oauth_application_settings + + expect(response).to have_gitlab_http_status(:internal_server_error) + end + + it 'returns 200 if oauth application exists' do + stub_application_setting({ + web_ide_oauth_application: create(:oauth_application, owner_id: nil, owner_type: nil) + }) + + reset_oauth_application_settings + + expect(response).to have_gitlab_http_status(:ok) + end + end end diff --git a/spec/frontend/ide/components/oauth_application_callout_spec.js b/spec/frontend/ide/components/oauth_application_callout_spec.js new file mode 100644 index 0000000000000..fd74c90789cd4 --- /dev/null +++ b/spec/frontend/ide/components/oauth_application_callout_spec.js @@ -0,0 +1,56 @@ +import { GlAlert, GlButton } from '@gitlab/ui'; +import { mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import WebIdeOAuthApplicationCallout, { + I18N_WEB_IDE_OAUTH_APPLICATION_CALLOUT, +} from '~/ide/components/oauth_application_callout.vue'; +import ResetApplicationSettingsModal from '~/ide/components/reset_application_settings_modal.vue'; + +const MOCK_IDE_REDIRECT_PATH = '/ide/oauth_redirect'; +const MOCK_IDE_RESET_APPLICATION_SETTINGS_PATH = '/ide/reset_application_settings'; + +describe('WebIdeOAuthApplicationCallout', () => { + let wrapper; + + const findAlert = () => wrapper.findComponent(GlAlert); + const findButton = () => wrapper.findComponent(GlButton); + const findModal = () => wrapper.findComponent(ResetApplicationSettingsModal); + + const createWrapper = () => { + wrapper = mount(WebIdeOAuthApplicationCallout, { + propsData: { + redirectUrlPath: MOCK_IDE_REDIRECT_PATH, + resetApplicationSettingsPath: MOCK_IDE_RESET_APPLICATION_SETTINGS_PATH, + }, + }); + }; + + beforeEach(() => { + createWrapper(); + }); + + it('renders alert', () => { + expect(findAlert().exists()).toBe(true); + expect(findButton().text()).toBe(I18N_WEB_IDE_OAUTH_APPLICATION_CALLOUT.alertButtonText); + }); + + it('shows reset application settings modal on restore button click', async () => { + findButton().vm.$emit('click'); + await nextTick(); + const modal = findModal(); + expect(modal.exists()).toBe(true); + expect(modal.props('visible')).toBe(true); + }); + + it.each(['close', 'cancel'])( + 'hides reset application settings modal on close or cancel', + async (eventName) => { + findModal().vm.$emit(eventName); + await nextTick(); + + const modal = findModal(); + expect(modal.exists()).toBe(true); + expect(modal.props('visible')).toBe(false); + }, + ); +}); diff --git a/spec/frontend/ide/components/reset_application_settings_modal_spec.js b/spec/frontend/ide/components/reset_application_settings_modal_spec.js new file mode 100644 index 0000000000000..ca818e541f424 --- /dev/null +++ b/spec/frontend/ide/components/reset_application_settings_modal_spec.js @@ -0,0 +1,118 @@ +import MockAdapter from 'axios-mock-adapter'; +import { GlAlert, GlModal } from '@gitlab/ui'; +import { mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; + +import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_NO_CONTENT, HTTP_STATUS_BAD_REQUEST } from '~/lib/utils/http_status'; +import ResetApplicationSettingsModal, { + I18N_RESET_APPLICATION_SETTINGS_MODAL, +} from '~/ide/components/reset_application_settings_modal.vue'; +import { useMockLocationHelper } from 'helpers/mock_window_location_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import { stubComponent } from 'helpers/stub_component'; + +const mockEvent = { preventDefault: jest.fn() }; +const mockHide = jest.fn(); +const MOCK_RESET_APPLICATION_SETTINGS_PATH = '/reset_application_settings_path'; + +describe('ResetApplicationSettingsModal', () => { + useMockLocationHelper(); + + let mockAxios; + let wrapper; + + const findModal = () => wrapper.findComponent(GlModal); + const findAlert = () => wrapper.findComponent(GlAlert); + const firePrimaryEvent = () => findModal().vm.$emit('primary', mockEvent); + + const createWrapper = () => { + wrapper = mount(ResetApplicationSettingsModal, { + propsData: { + visible: true, + resetApplicationSettingsPath: MOCK_RESET_APPLICATION_SETTINGS_PATH, + }, + stubs: { + GlModal: stubComponent(GlModal, { + methods: { + hide: mockHide, + }, + }), + }, + }); + }; + + beforeEach(() => { + mockAxios = new MockAdapter(axios); + + createWrapper(); + }); + + afterEach(() => { + mockAxios.restore(); + }); + + it('renders modal with correct props', () => { + const modal = findModal(); + + expect(modal.props('modalId')).toBe('reset-application-settings-modal'); + expect(modal.props('title')).toEqual(I18N_RESET_APPLICATION_SETTINGS_MODAL.title); + expect(modal.props('visible')).toBe(true); + }); + + describe('resetApplicationSetings', () => { + it('makes request to reset application settings path', async () => { + mockAxios.onPost(MOCK_RESET_APPLICATION_SETTINGS_PATH).reply(HTTP_STATUS_NO_CONTENT); + + firePrimaryEvent(); + await waitForPromises(); + await nextTick(); + + expect(mockAxios.history.post.length).toBe(1); + expect(mockAxios.history.post[0].url).toBe(MOCK_RESET_APPLICATION_SETTINGS_PATH); + }); + + describe('on success', () => { + beforeEach(async () => { + mockAxios.onPost(MOCK_RESET_APPLICATION_SETTINGS_PATH).reply(HTTP_STATUS_NO_CONTENT); + + firePrimaryEvent(); + await waitForPromises(); + await nextTick(); + }); + + it('closes modal and reloads window upon successful reset', () => { + expect(mockHide).toHaveBeenCalledTimes(1); + expect(window.location.reload).toHaveBeenCalledTimes(1); + }); + + it('does not display error alert', () => { + expect(findAlert().exists()).toBe(false); + }); + }); + + describe('on error', () => { + beforeEach(async () => { + mockAxios.onPost(MOCK_RESET_APPLICATION_SETTINGS_PATH).reply(HTTP_STATUS_BAD_REQUEST); + + firePrimaryEvent(); + await waitForPromises(); + await nextTick(); + }); + + it('does not reload window upon error', () => { + expect(window.location.reload).not.toHaveBeenCalled(); + }); + + it('displays error alert', () => { + const modal = findModal(); + const errorAlert = findAlert(); + + expect(modal.props('visible')).toBe(true); + expect(modal.props('actionPrimary').attributes.loading).toBe(false); + expect(errorAlert.exists()).toBe(true); + expect(errorAlert.text()).toBe(I18N_RESET_APPLICATION_SETTINGS_MODAL.errorMessage); + }); + }); + }); +}); diff --git a/spec/lib/web_ide/default_oauth_application_spec.rb b/spec/lib/web_ide/default_oauth_application_spec.rb index 4f051ffaafbed..dc2359e771da8 100644 --- a/spec/lib/web_ide/default_oauth_application_spec.rb +++ b/spec/lib/web_ide/default_oauth_application_spec.rb @@ -110,6 +110,26 @@ end end + describe '#reset_oauth_application_settings' do + it 'resets oauth application settings to original' do + mock_bad_oauth_application = oauth_application + mock_bad_oauth_application["confidential"] = true + mock_bad_oauth_application["trusted"] = false + + stub_application_setting({ web_ide_oauth_application: mock_bad_oauth_application }) + + described_class.reset_oauth_application_settings + + expect(oauth_application).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 -- GitLab