Migrate Axios CAPTCHA Interceptor to use headers

- See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64463

app/assets/javascripts/captcha/captcha_modal_axios_interceptor.js

-const supportedMethods = ['patch', 'post', 'put'];
+const SUPPORTED_METHODS = ['patch', 'post', 'put'];
+
+function needsCaptchaResponse(err) {
+  return (
+    SUPPORTED_METHODS.includes(err?.config?.method) && err?.response?.data?.needs_captcha_response
+  );
+}
+
+const showCaptchaModalAndResubmit = async (axios, data, errConfig) => {
+  // NOTE: We asynchronously import and unbox the module. Since this is included globally, we don't
+  // do a regular import because that would increase the size of the webpack bundle.
+  const { waitForCaptchaToBeSolved } = await import('~/captcha/wait_for_captcha_to_be_solved');
+
+  // show the CAPTCHA modal and wait for it to be solved or closed
+  const captchaResponse = await waitForCaptchaToBeSolved(data.captcha_site_key);
+
+  // resubmit the original request with the captcha_response and spam_log_id in the headers
+  const originalData = JSON.parse(errConfig.data);
+  const originalHeaders = errConfig.headers;
+  return axios({
+    method: errConfig.method,
+    url: errConfig.url,
+    headers: {
+      ...originalHeaders,
+      'X-GitLab-Captcha-Response': captchaResponse,
+      'X-GitLab-Spam-Log-Id': data.spam_log_id,
+    },
+    data: originalData,
+  });
+};
 
 export function registerCaptchaModalInterceptor(axios) {
   return axios.interceptors.response.use(
@@ -6,29 +35,8 @@ export function registerCaptchaModalInterceptor(axios) {
       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,
-              },
-            });
-          });
+      if (needsCaptchaResponse(err)) {
+        return showCaptchaModalAndResubmit(axios, err.response.data, err.config);
       }
 
       return Promise.reject(err);

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/axios_utils.js

 import axios from 'axios';
+import { registerCaptchaModalInterceptor } from '~/captcha/captcha_modal_axios_interceptor';
 import setupAxiosStartupCalls from './axios_startup_calls';
 import csrf from './csrf';
 import suppressAjaxErrorsDuringNavigation from './suppress_ajax_errors_during_navigation';
@@ -41,6 +42,8 @@ axios.interceptors.response.use(
   (err) => suppressAjaxErrorsDuringNavigation(err, isUserNavigating),
 );
 
+registerCaptchaModalInterceptor(axios);
+
 export default axios;
 
 /**

app/controllers/concerns/spammable_actions.rb

@@ -47,17 +47,13 @@ module SpammableActions
     end
   end
 
-  # TODO: This method is currently only needed for issue create and update. It can be removed when:
-  #
-  # 1. Issue create is is converted to a client/JS based approach instead of the legacy HAML
-  #    `_recaptcha_form.html.haml` which is rendered via the `projects/issues/verify` template.
-  #    In this case, which is based on the legacy reCAPTCHA implementation using the HTML/HAML form,
-  #    the 'g-recaptcha-response' field name comes from `Recaptcha::ClientHelper#recaptcha_tags` in the
-  #    recaptcha gem, which is called from the HAML `_recaptcha_form.html.haml` form.
-  # 2. Issue update is converted to use the headers-based approach, which will require adding
-  #    support to captcha_modal_axios_interceptor.js like we have already added to
-  #    apollo_captcha_link.js.
-  #    In this case, the `captcha_response` field name comes from our captcha_modal_axios_interceptor.js.
+  # TODO: This method is currently only needed for issue create, to convert spam/CAPTCHA values from
+  #   params, and instead be passed as headers, as the spam services now all expect. It can be removed
+  #   when issue create is is converted to a client/JS based approach instead of the legacy HAML
+  #   `_recaptcha_form.html.haml` which is rendered via the `projects/issues/verify` template.
+  #   In that case, which is based on the legacy reCAPTCHA implementation using the HTML/HAML form,
+  #   the 'g-recaptcha-response' field name comes from `Recaptcha::ClientHelper#recaptcha_tags` in the
+  #   recaptcha gem, which is called from the HAML `_recaptcha_form.html.haml` form.
   def extract_legacy_spam_params_to_headers
     request.headers['X-GitLab-Captcha-Response'] = params['g-recaptcha-response'] || params[:captcha_response]
     request.headers['X-GitLab-Spam-Log-Id'] = params[:spam_log_id]

app/controllers/projects/issues_controller.rb

@@ -336,7 +336,6 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def update_service
-    extract_legacy_spam_params_to_headers
     spam_params = ::Spam::SpamParams.new_from_request(request: request)
     ::Issues::UpdateService.new(project: project, current_user: current_user, params: issue_params, spam_params: spam_params)
   end

ee/spec/frontend/boards/components/boards_selector_spec.js

@@ -139,8 +139,11 @@ describe('BoardsSelector', () => {
       await wrapper.setData({
         loadingBoards: false,
       });
-      await Promise.all([allBoardsResponse, recentBoardsResponse]);
-      return nextTick();
+      // NOTE: Due to timing issues, this `return` of `Promise.all` is required because
+      // `app/assets/javascripts/boards/components/boards_selector.vue` does a `$nextTick()` in
+      // loadRecentBoards. For some unknown reason it doesn't work with `await`, the `return`
+      // is required.
+      return Promise.all([allBoardsResponse, recentBoardsResponse]).then(() => nextTick());
     });
 
     it('hides loading spinner', async () => {

spec/controllers/projects/issues_controller_spec.rb

@@ -1016,10 +1016,13 @@ RSpec.describe Projects::IssuesController do
             let(:spammy_title) { 'Whatever' }
             let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) }
 
+            before do
+              request.headers['X-GitLab-Captcha-Response'] = 'a-valid-captcha-response'
+              request.headers['X-GitLab-Spam-Log-Id'] = spam_logs.last.id
+            end
+
             def update_verified_issue
-              update_issue(
-                issue_params: { title: spammy_title },
-                additional_params: { spam_log_id: spam_logs.last.id, 'g-recaptcha-response': true })
+              update_issue(issue_params: { title: spammy_title })
             end
 
             it 'returns 200 status' do
@@ -1036,8 +1039,9 @@ RSpec.describe Projects::IssuesController do
 
             it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do
               spam_log = create(:spam_log)
+              request.headers['X-GitLab-Spam-Log-Id'] = spam_log.id
 
-              expect { update_issue(issue_params: { spam_log_id: spam_log.id, 'g-recaptcha-response': true }) }
+              expect { update_issue }
                 .not_to change { SpamLog.last.recaptcha_verified }
             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 UnsolvedCaptchaError from '~/captcha/unsolved_captcha_error';
 import { waitForCaptchaToBeSolved } from '~/captcha/wait_for_captcha_to_be_solved';
 import axios from '~/lib/utils/axios_utils';
 import httpStatusCodes from '~/lib/utils/http_status';
@@ -25,22 +26,24 @@ describe('registerCaptchaModalInterceptor', () => {
   let mock;
 
   beforeEach(() => {
+    waitForCaptchaToBeSolved.mockRejectedValue(new UnsolvedCaptchaError());
+
     mock = new MockAdapter(axios);
-    mock.onAny('/no-captcha').reply(200, AXIOS_RESPONSE);
-    mock.onAny('/error').reply(404, AXIOS_RESPONSE);
-    mock.onAny('/captcha').reply((config) => {
+    mock.onAny('/endpoint-without-captcha').reply(200, AXIOS_RESPONSE);
+    mock.onAny('/endpoint-with-unrelated-error').reply(404, AXIOS_RESPONSE);
+    mock.onAny('/endpoint-with-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 }];
+      const data = JSON.parse(config.data);
+      const {
+        'X-GitLab-Captcha-Response': captchaResponse,
+        'X-GitLab-Spam-Log-Id': spamLogId,
+      } = config.headers;
+
+      if (captchaResponse === CAPTCHA_RESPONSE && spamLogId === SPAM_LOG_ID) {
+        return [httpStatusCodes.OK, { ...data, method: config.method, CAPTCHA_SUCCESS }];
       }
 
       return [httpStatusCodes.CONFLICT, NEEDS_CAPTCHA_RESPONSE];
@@ -56,7 +59,7 @@ describe('registerCaptchaModalInterceptor', () => {
 
   describe.each([...supportedMethods, ...unsupportedMethods])('For HTTP method %s', (method) => {
     it('successful requests are passed through', async () => {
-      const { data, status } = await axios[method]('/no-captcha');
+      const { data, status } = await axios[method]('/endpoint-without-captcha');
 
       expect(status).toEqual(httpStatusCodes.OK);
       expect(data).toEqual(AXIOS_RESPONSE);
@@ -64,7 +67,7 @@ describe('registerCaptchaModalInterceptor', () => {
     });
 
     it('error requests without needs_captcha_response_errors are passed through', async () => {
-      await expect(() => axios[method]('/error')).rejects.toThrow(
+      await expect(() => axios[method]('/endpoint-with-unrelated-error')).rejects.toThrow(
         expect.objectContaining({
           response: expect.objectContaining({
             status: httpStatusCodes.NOT_FOUND,
@@ -79,21 +82,35 @@ describe('registerCaptchaModalInterceptor', () => {
   describe.each(supportedMethods)('For HTTP method %s', (method) => {
     describe('error requests with needs_captcha_response_errors', () => {
       const submittedData = { ID: 12345 };
+      const submittedHeaders = { 'Submitted-Header': 67890 };
 
       it('re-submits request if captcha was solved correctly', async () => {
-        waitForCaptchaToBeSolved.mockResolvedValue(CAPTCHA_RESPONSE);
-        const { data: returnedData } = await axios[method]('/captcha', submittedData);
+        waitForCaptchaToBeSolved.mockResolvedValueOnce(CAPTCHA_RESPONSE);
+        const axiosResponse = await axios[method]('/endpoint-with-captcha', submittedData, {
+          headers: submittedHeaders,
+        });
+        const {
+          data: returnedData,
+          config: { headers: returnedHeaders },
+        } = axiosResponse;
 
         expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY);
 
         expect(returnedData).toEqual({ ...submittedData, CAPTCHA_SUCCESS, method });
+        expect(returnedHeaders).toEqual(
+          expect.objectContaining({
+            ...submittedHeaders,
+            'X-GitLab-Captcha-Response': CAPTCHA_RESPONSE,
+            'X-GitLab-Spam-Log-Id': SPAM_LOG_ID,
+          }),
+        );
         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);
+        await expect(() => axios[method]('/endpoint-with-captcha', submittedData)).rejects.toThrow(
+          new UnsolvedCaptchaError(),
+        );
 
         expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY);
         expect(mock.history[method]).toHaveLength(1);
@@ -103,7 +120,7 @@ describe('registerCaptchaModalInterceptor', () => {
 
   describe.each(unsupportedMethods)('For HTTP method %s', (method) => {
     it('ignores captcha response', async () => {
-      await expect(() => axios[method]('/captcha')).rejects.toThrow(
+      await expect(() => axios[method]('/endpoint-with-captcha')).rejects.toThrow(
         expect.objectContaining({
           response: expect.objectContaining({
             status: httpStatusCodes.METHOD_NOT_ALLOWED,