Merge branch 'convert-issues-to-new-captcha-modal' into 'master'

Add support for new CAPTCHA modal to issue updates

See merge request gitlab-org/gitlab!53941

app/assets/javascripts/captcha/captcha_modal.vue

@@ -41,10 +41,17 @@ export default {
       }
     },
   },
+  mounted() {
+    // If this is true, we need to present the captcha modal to the user.
+    // When the modal is shown we will also initialize and render the form.
+    if (this.needsCaptchaResponse) {
+      this.$refs.modal.show();
+    }
+  },
   methods: {
     emitReceivedCaptchaResponse(captchaResponse) {
-      this.$emit('receivedCaptchaResponse', captchaResponse);
       this.$refs.modal.hide();
+      this.$emit('receivedCaptchaResponse', captchaResponse);
     },
     emitNullReceivedCaptchaResponse() {
       this.emitReceivedCaptchaResponse(null);
@@ -103,6 +110,7 @@ export default {
     :action-cancel="{ text: __('Cancel') }"
     @shown="shown"
     @hide="hide"
+    @hidden="$emit('hidden')"
   >
     <div ref="captcha"></div>
     <p>{{ __('We want to be sure it is you, please confirm you are not a robot.') }}</p>

app/assets/javascripts/captcha/captcha_modal_axios_interceptor.js

+const supportedMethods = ['patch', 'post', 'put'];
+
+export function registerCaptchaModalInterceptor(axios) {
+  return axios.interceptors.response.use(
+    (response) => {
+      return response;
+    },
+    (err) => {
+      if (
+        supportedMethods.includes(err?.config?.method) &&
+        err?.response?.data?.needs_captcha_response
+      ) {
+        const { data } = err.response;
+        const captchaSiteKey = data.captcha_site_key;
+        const spamLogId = data.spam_log_id;
+        // eslint-disable-next-line promise/no-promise-in-callback
+        return import('~/captcha/wait_for_captcha_to_be_solved')
+          .then(({ waitForCaptchaToBeSolved }) => waitForCaptchaToBeSolved(captchaSiteKey))
+          .then((captchaResponse) => {
+            const errConfig = err.config;
+            const originalData = JSON.parse(errConfig.data);
+            return axios({
+              method: errConfig.method,
+              url: errConfig.url,
+              data: {
+                ...originalData,
+                captcha_response: captchaResponse,
+                spam_log_id: spamLogId,
+              },
+            });
+          });
+      }
+
+      return Promise.reject(err);
+    },
+  );
+}

app/assets/javascripts/captcha/unsolved_captcha_error.js

+import { __ } from '~/locale';
+
+class UnsolvedCaptchaError extends Error {
+  constructor(message) {
+    super(message || __('You must solve the CAPTCHA in order to submit'));
+    this.name = 'UnsolvedCaptchaError';
+  }
+}
+
+export default UnsolvedCaptchaError;

app/assets/javascripts/captcha/wait_for_captcha_to_be_solved.js

+import Vue from 'vue';
+import CaptchaModal from '~/captcha/captcha_modal.vue';
+import UnsolvedCaptchaError from '~/captcha/unsolved_captcha_error';
+
+/**
+ * Opens a Captcha Modal with provided captchaSiteKey.
+ *
+ * Returns a Promise which resolves if the captcha is solved correctly, and rejects
+ * if the captcha process is aborted.
+ *
+ * @param captchaSiteKey
+ * @returns {Promise}
+ */
+export function waitForCaptchaToBeSolved(captchaSiteKey) {
+  return new Promise((resolve, reject) => {
+    let captchaModalElement = document.createElement('div');
+
+    document.body.append(captchaModalElement);
+
+    let captchaModalVueInstance = new Vue({
+      el: captchaModalElement,
+      render: (createElement) => {
+        return createElement(CaptchaModal, {
+          props: {
+            captchaSiteKey,
+            needsCaptchaResponse: true,
+          },
+          on: {
+            hidden: () => {
+              // Cleaning up the modal from the DOM
+              captchaModalVueInstance.$destroy();
+              captchaModalVueInstance.$el.remove();
+              captchaModalElement.remove();
+
+              captchaModalElement = null;
+              captchaModalVueInstance = null;
+            },
+            receivedCaptchaResponse: (captchaResponse) => {
+              if (captchaResponse) {
+                resolve(captchaResponse);
+              } else {
+                // reject the promise with a custom exception, allowing consuming apps to
+                // adjust their error handling, if appropriate.
+                const error = new UnsolvedCaptchaError();
+                reject(error);
+              }
+            },
+          },
+        });
+      },
+    });
+  });
+}

app/assets/javascripts/issue_show/components/app.vue

@@ -5,7 +5,6 @@ import { deprecatedCreateFlash as createFlash } from '~/flash';
 import Poll from '~/lib/utils/poll';
 import { visitUrl } from '~/lib/utils/url_utility';
 import { __, s__, sprintf } from '~/locale';
-import recaptchaModalImplementor from '~/vue_shared/mixins/recaptcha_modal_implementor';
 import { IssuableStatus, IssuableStatusText, IssuableType } from '../constants';
 import eventHub from '../event_hub';
 import Service from '../services/index';
@@ -25,7 +24,6 @@ export default {
     formComponent,
     PinnedLinks,
   },
-  mixins: [recaptchaModalImplementor],
   props: {
     endpoint: {
       required: true,
@@ -250,6 +248,7 @@ export default {
     },
   },
   created() {
+    this.flashContainer = null;
     this.service = new Service(this.endpoint);
     this.poll = new Poll({
       resource: this.service,
@@ -289,7 +288,7 @@ export default {
   methods: {
     handleBeforeUnloadEvent(e) {
       const event = e;
-      if (this.showForm && this.issueChanged && !this.showRecaptcha) {
+      if (this.showForm && this.issueChanged) {
         event.returnValue = __('Are you sure you want to lose your issue information?');
       }
       return undefined;
@@ -347,10 +346,10 @@ export default {
     },
 
     updateIssuable() {
+      this.clearFlash();
       return this.service
         .updateIssuable(this.store.formState)
         .then((res) => res.data)
-        .then((data) => this.checkForSpam(data))
         .then((data) => {
           if (!window.location.pathname.includes(data.web_url)) {
             visitUrl(data.web_url);
@@ -361,28 +360,22 @@ export default {
           eventHub.$emit('close.form');
         })
         .catch((error = {}) => {
-          const { name, response = {} } = error;
+          const { message, response = {} } = error;
 
-          if (name === 'SpamError') {
-            this.openRecaptcha();
-          } else {
-            let errMsg = this.defaultErrorMessage;
+          this.store.setFormState({
+            updateLoading: false,
+          });
 
-            if (response.data && response.data.errors) {
-              errMsg += `. ${response.data.errors.join(' ')}`;
-            }
+          let errMsg = this.defaultErrorMessage;
 
-            createFlash(errMsg);
+          if (response.data && response.data.errors) {
+            errMsg += `. ${response.data.errors.join(' ')}`;
+          } else if (message) {
+            errMsg += `. ${message}`;
           }
-        });
-    },
-
-    closeRecaptchaModal() {
-      this.store.setFormState({
-        updateLoading: false,
-      });
 
-      this.closeRecaptcha();
+          this.flashContainer = createFlash(errMsg);
+        });
     },
 
     deleteIssuable(payload) {
@@ -409,6 +402,13 @@ export default {
     showStickyHeader() {
       this.isStickyHeaderShowing = true;
     },
+
+    clearFlash() {
+      if (this.flashContainer) {
+        this.flashContainer.style.display = 'none';
+        this.flashContainer = null;
+      }
+    },
   },
 };
 </script>
@@ -430,13 +430,6 @@ export default {
         :enable-autocomplete="enableAutocomplete"
         :issuable-type="issuableType"
       />
-
-      <recaptcha-modal
-        v-show="showRecaptcha"
-        ref="recaptchaModal"
-        :html="recaptchaHTML"
-        @close="closeRecaptchaModal"
-      />
     </div>
     <div v-else>
       <title-component

app/assets/javascripts/issue_show/services/index.js

+import { registerCaptchaModalInterceptor } from '~/captcha/captcha_modal_axios_interceptor';
 import axios from '../../lib/utils/axios_utils';
 
 export default class Service {
   constructor(endpoint) {
     this.endpoint = `${endpoint}.json`;
     this.realtimeEndpoint = `${endpoint}/realtime_changes`;
+    registerCaptchaModalInterceptor(axios);
   }
 
   getData() {

app/assets/javascripts/lib/utils/http_status.js

@@ -19,6 +19,7 @@ const httpStatusCodes = {
   UNAUTHORIZED: 401,
   FORBIDDEN: 403,
   NOT_FOUND: 404,
+  METHOD_NOT_ALLOWED: 405,
   CONFLICT: 409,
   GONE: 410,
   UNPROCESSABLE_ENTITY: 422,

app/assets/javascripts/snippets/components/edit.vue

@@ -221,7 +221,10 @@ export default {
       this.captchaResponse = captchaResponse;
 
       if (this.captchaResponse) {
-        // If the user solved the captcha resubmit the form.
+        // If the user solved the captcha, resubmit the form.
+        // NOTE: we do not need to clear out the captchaResponse and spamLogId
+        // data values after submit, because this component always does a full page reload.
+        // Otherwise, we would need to.
         this.handleFormSubmit();
       } else {
         // If the user didn't solve the captcha (e.g. they just closed the modal),

app/controllers/concerns/spammable_actions.rb

@@ -2,6 +2,7 @@
 
 module SpammableActions
   extend ActiveSupport::Concern
+  include Spam::Concerns::HasSpamActionResponseFields
 
   included do
     before_action :authorize_submit_spammable!, only: :mark_as_spam
@@ -25,14 +26,20 @@ module SpammableActions
 
       respond_to do |format|
         format.html do
+          # NOTE: format.html is still used by issue create, and uses the legacy HAML
+          # `_recaptcha_form.html.haml` rendered via the `projects/issues/verify` template.
           render :verify
         end
 
         format.json do
-          locals = { spammable: spammable, script: false, has_submit: false }
-          recaptcha_html = render_to_string(partial: 'shared/recaptcha_form', formats: :html, locals: locals)
+          # format.json is used by all new Vue-based CAPTCHA implementations, which
+          # handle all of the CAPTCHA form rendering on the client via the Pajamas-based
+          # app/assets/javascripts/captcha/captcha_modal.vue
 
-          render json: { recaptcha_html: recaptcha_html }
+          # NOTE: "409 - Conflict" seems to be the most appropriate HTTP status code for a response
+          # which requires a CAPTCHA to be solved in order for the request to be resubmitted.
+          # See https://stackoverflow.com/q/26547466/25192
+          render json: spam_action_response_fields(spammable), status: :conflict
         end
       end
     else
@@ -58,7 +65,7 @@ module SpammableActions
 
     # After this newer GraphQL/JS API process is fully supported by the backend, we can remove the
     # check for the 'g-recaptcha-response' field and other HTML/HAML form-specific support.
-    captcha_response = params['g-recaptcha-response']
+    captcha_response = params['g-recaptcha-response'] || params[:captcha_response]
 
     {
       request: request,

locale/gitlab.pot

@@ -34456,6 +34456,9 @@ msgstr ""
 msgid "You must set up incoming email before it becomes active."
 msgstr ""
 
+msgid "You must solve the CAPTCHA in order to submit"
+msgstr ""
+
 msgid "You must upload a file with the same file name when dropping onto an existing design."
 msgstr ""
 

spec/controllers/concerns/spammable_actions_spec.rb

@@ -69,8 +69,11 @@ RSpec.describe SpammableActions do
       end
 
       context 'when spammable.render_recaptcha? is true' do
+        let(:spam_log) { instance_double(SpamLog, id: 123) }
+        let(:captcha_site_key) { 'abc123' }
+
         before do
-          expect(spammable).to receive(:render_recaptcha?) { true }
+          expect(spammable).to receive(:render_recaptcha?).at_least(:once) { true }
         end
 
         context 'when format is :html' do
@@ -83,24 +86,24 @@ RSpec.describe SpammableActions do
 
         context 'when format is :json' do
           let(:format) { :json }
-          let(:recaptcha_html) { '<recaptcha-html/>' }
 
-          it 'renders json with recaptcha_html' do
-            expect(controller).to receive(:render_to_string).with(
-              {
-                partial: 'shared/recaptcha_form',
-                formats: :html,
-                locals: {
-                  spammable: spammable,
-                  script: false,
-                  has_submit: false
-                }
-              }
-            ) { recaptcha_html }
+          before do
+            expect(spammable).to receive(:spam?) { false }
+            expect(spammable).to receive(:spam_log) { spam_log }
+            expect(Gitlab::CurrentSettings).to receive(:recaptcha_site_key) { captcha_site_key }
+          end
 
+          it 'renders json with spam_action_response_fields' do
             subject
 
-            expect(json_response).to eq({ 'recaptcha_html' => recaptcha_html })
+            expected_json_response = HashWithIndifferentAccess.new(
+              {
+                spam: false,
+                needs_captcha_response: true,
+                spam_log_id: spam_log.id,
+                captcha_site_key: captcha_site_key
+              })
+            expect(json_response).to eq(expected_json_response)
           end
         end
       end

spec/controllers/projects/issues_controller_spec.rb

@@ -9,6 +9,7 @@ RSpec.describe Projects::IssuesController do
   let_it_be(:project, reload: true) { create(:project) }
   let_it_be(:user, reload: true) { create(:user) }
   let(:issue) { create(:issue, project: project) }
+  let(:spam_action_response_fields) { { 'stub_spam_action_response_fields' => true } }
 
   describe "GET #index" do
     context 'external issue tracker' do
@@ -613,12 +614,15 @@ RSpec.describe Projects::IssuesController do
         context 'when allow_possible_spam feature flag is false' do
           before do
             stub_feature_flags(allow_possible_spam: false)
+            expect(controller).to(receive(:spam_action_response_fields).with(issue)) do
+              spam_action_response_fields
+            end
           end
 
-          it 'renders json with recaptcha_html' do
+          it 'renders json with spam_action_response_fields' do
             subject
 
-            expect(json_response).to have_key('recaptcha_html')
+            expect(json_response).to eq(spam_action_response_fields)
           end
         end
 
@@ -948,12 +952,17 @@ RSpec.describe Projects::IssuesController do
               context 'renders properly' do
                 render_views
 
-                it 'renders recaptcha_html json response' do
+                before do
+                  expect(controller).to(receive(:spam_action_response_fields).with(issue)) do
+                    spam_action_response_fields
+                  end
+                end
+
+                it 'renders spam_action_response_fields json response' do
                   update_issue
 
-                  expect(response).to have_gitlab_http_status(:ok)
-                  expect(json_response).to have_key('recaptcha_html')
-                  expect(json_response['recaptcha_html']).not_to be_empty
+                  expect(response).to have_gitlab_http_status(:conflict)
+                  expect(json_response).to eq(spam_action_response_fields)
                 end
               end
             end

spec/frontend/captcha/captcha_modal_axios_interceptor_spec.js

+import MockAdapter from 'axios-mock-adapter';
+
+import { registerCaptchaModalInterceptor } from '~/captcha/captcha_modal_axios_interceptor';
+import { waitForCaptchaToBeSolved } from '~/captcha/wait_for_captcha_to_be_solved';
+import axios from '~/lib/utils/axios_utils';
+import httpStatusCodes from '~/lib/utils/http_status';
+
+jest.mock('~/captcha/wait_for_captcha_to_be_solved');
+
+describe('registerCaptchaModalInterceptor', () => {
+  const SPAM_LOG_ID = 'SPAM_LOG_ID';
+  const CAPTCHA_SITE_KEY = 'CAPTCHA_SITE_KEY';
+  const CAPTCHA_SUCCESS = 'CAPTCHA_SUCCESS';
+  const CAPTCHA_RESPONSE = 'CAPTCHA_RESPONSE';
+  const AXIOS_RESPONSE = { text: 'AXIOS_RESPONSE' };
+  const NEEDS_CAPTCHA_RESPONSE = {
+    needs_captcha_response: true,
+    captcha_site_key: CAPTCHA_SITE_KEY,
+    spam_log_id: SPAM_LOG_ID,
+  };
+
+  const unsupportedMethods = ['delete', 'get', 'head', 'options'];
+  const supportedMethods = ['patch', 'post', 'put'];
+
+  let mock;
+
+  beforeEach(() => {
+    mock = new MockAdapter(axios);
+    mock.onAny('/no-captcha').reply(200, AXIOS_RESPONSE);
+    mock.onAny('/error').reply(404, AXIOS_RESPONSE);
+    mock.onAny('/captcha').reply((config) => {
+      if (!supportedMethods.includes(config.method)) {
+        return [httpStatusCodes.METHOD_NOT_ALLOWED, { method: config.method }];
+      }
+
+      try {
+        const { captcha_response, spam_log_id, ...rest } = JSON.parse(config.data);
+        // eslint-disable-next-line babel/camelcase
+        if (captcha_response === CAPTCHA_RESPONSE && spam_log_id === SPAM_LOG_ID) {
+          return [httpStatusCodes.OK, { ...rest, method: config.method, CAPTCHA_SUCCESS }];
+        }
+      } catch (e) {
+        return [httpStatusCodes.BAD_REQUEST, { method: config.method }];
+      }
+
+      return [httpStatusCodes.CONFLICT, NEEDS_CAPTCHA_RESPONSE];
+    });
+
+    axios.interceptors.response.handlers = [];
+    registerCaptchaModalInterceptor(axios);
+  });
+
+  afterEach(() => {
+    mock.restore();
+  });
+
+  describe.each([...supportedMethods, ...unsupportedMethods])('For HTTP method %s', (method) => {
+    it('successful requests are passed through', async () => {
+      const { data, status } = await axios[method]('/no-captcha');
+
+      expect(status).toEqual(httpStatusCodes.OK);
+      expect(data).toEqual(AXIOS_RESPONSE);
+      expect(mock.history[method]).toHaveLength(1);
+    });
+
+    it('error requests without needs_captcha_response_errors are passed through', async () => {
+      await expect(() => axios[method]('/error')).rejects.toThrow(
+        expect.objectContaining({
+          response: expect.objectContaining({
+            status: httpStatusCodes.NOT_FOUND,
+            data: AXIOS_RESPONSE,
+          }),
+        }),
+      );
+      expect(mock.history[method]).toHaveLength(1);
+    });
+  });
+
+  describe.each(supportedMethods)('For HTTP method %s', (method) => {
+    describe('error requests with needs_captcha_response_errors', () => {
+      const submittedData = { ID: 12345 };
+
+      it('re-submits request if captcha was solved correctly', async () => {
+        waitForCaptchaToBeSolved.mockResolvedValue(CAPTCHA_RESPONSE);
+        const { data: returnedData } = await axios[method]('/captcha', submittedData);
+
+        expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY);
+
+        expect(returnedData).toEqual({ ...submittedData, CAPTCHA_SUCCESS, method });
+        expect(mock.history[method]).toHaveLength(2);
+      });
+
+      it('does not re-submit request if captcha was not solved', async () => {
+        const error = new Error('Captcha not solved');
+        waitForCaptchaToBeSolved.mockRejectedValue(error);
+        await expect(() => axios[method]('/captcha', submittedData)).rejects.toThrow(error);
+
+        expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY);
+        expect(mock.history[method]).toHaveLength(1);
+      });
+    });
+  });
+
+  describe.each(unsupportedMethods)('For HTTP method %s', (method) => {
+    it('ignores captcha response', async () => {
+      await expect(() => axios[method]('/captcha')).rejects.toThrow(
+        expect.objectContaining({
+          response: expect.objectContaining({
+            status: httpStatusCodes.METHOD_NOT_ALLOWED,
+            data: { method },
+          }),
+        }),
+      );
+
+      expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled();
+      expect(mock.history[method]).toHaveLength(1);
+    });
+  });
+});

spec/frontend/captcha/wait_for_captcha_to_be_solved_spec.js

+import CaptchaModal from '~/captcha/captcha_modal.vue';
+import { waitForCaptchaToBeSolved } from '~/captcha/wait_for_captcha_to_be_solved';
+
+jest.mock('~/captcha/captcha_modal.vue', () => ({
+  mounted: jest.fn(),
+  render(h) {
+    return h('div', { attrs: { id: 'mock-modal' } });
+  },
+}));
+
+describe('waitForCaptchaToBeSolved', () => {
+  const response = 'CAPTCHA_RESPONSE';
+
+  const findModal = () => document.querySelector('#mock-modal');
+
+  it('opens a modal, resolves with captcha response on success', async () => {
+    CaptchaModal.mounted.mockImplementationOnce(function mounted() {
+      requestAnimationFrame(() => {
+        this.$emit('receivedCaptchaResponse', response);
+        this.$emit('hidden');
+      });
+    });
+
+    expect(findModal()).toBeNull();
+
+    const promise = waitForCaptchaToBeSolved('FOO');
+
+    expect(findModal()).not.toBeNull();
+
+    const result = await promise;
+    expect(result).toEqual(response);
+
+    expect(findModal()).toBeNull();
+    expect(document.body.innerHTML).toEqual('');
+  });
+
+  it("opens a modal, rejects with error in case the captcha isn't solved", async () => {
+    CaptchaModal.mounted.mockImplementationOnce(function mounted() {
+      requestAnimationFrame(() => {
+        this.$emit('receivedCaptchaResponse', null);
+        this.$emit('hidden');
+      });
+    });
+
+    expect(findModal()).toBeNull();
+
+    const promise = waitForCaptchaToBeSolved('FOO');
+
+    expect(findModal()).not.toBeNull();
+
+    await expect(promise).rejects.toThrow(/You must solve the CAPTCHA in order to submit/);
+
+    expect(findModal()).toBeNull();
+    expect(document.body.innerHTML).toEqual('');
+  });
+});

spec/frontend/issue_show/components/app_spec.js

@@ -166,40 +166,6 @@ describe('Issuable output', () => {
     });
   });
 
-  it('opens reCAPTCHA modal if update rejected as spam', () => {
-    let modal;
-
-    jest.spyOn(wrapper.vm.service, 'updateIssuable').mockResolvedValue({
-      data: {
-        recaptcha_html: '<div class="g-recaptcha">recaptcha_html</div>',
-      },
-    });
-
-    wrapper.vm.canUpdate = true;
-    wrapper.vm.showForm = true;
-
-    return wrapper.vm
-      .$nextTick()
-      .then(() => {
-        wrapper.vm.$refs.recaptchaModal.scriptSrc = '//scriptsrc';
-        return wrapper.vm.updateIssuable();
-      })
-      .then(() => {
-        modal = wrapper.find('.js-recaptcha-modal');
-        expect(modal.isVisible()).toBe(true);
-        expect(modal.find('.g-recaptcha').text()).toEqual('recaptcha_html');
-        expect(document.body.querySelector('.js-recaptcha-script').src).toMatch('//scriptsrc');
-      })
-      .then(() => {
-        modal.find('.close').trigger('click');
-        return wrapper.vm.$nextTick();
-      })
-      .then(() => {
-        expect(modal.isVisible()).toBe(false);
-        expect(document.body.querySelector('.js-recaptcha-script')).toBeNull();
-      });
-  });
-
   describe('Pinned links propagated', () => {
     it.each`
       prop                      | value