Skip to content

G
gitlab-ce
Commit 42da5761 authored by Gabriel Mazetto's avatar Gabriel Mazetto
Browse files
Options

Merge branch 'axios-captcha-interceptor' into 'master' 

Migrate Axios Captcha Interceptor to be globally registered and using headers as well [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!64463
parents 80a79cbb 8b3e6f8b
Hide whitespace changes
Inline Side-by-side
Showing with 91 additions and 63 deletions
app/assets/javascripts/captcha/captcha_modal_axios_interceptor.js
View file @ 42da5761
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
View file @ 42da5761
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
View file @ 42da5761
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
View file @ 42da5761
......@@ -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
View file @ 42da5761
......@@ -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
View file @ 42da5761
......@@ -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
View file @ 42da5761
......@@ -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
View file @ 42da5761
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,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7