Commit 24b8ba7b authored by Chad Woolley's avatar Chad Woolley Committed by Lukas Eipert

Add support for new CAPTCHA modal to issue updates

Switch to new modern captcha modal which uses Pajamas modal and handles
all CAPTCHA rendering on the client. This introduces new, more decoupled
approach of using axios interceptors to handle captcha modal hooks.

The backend now returns a 409 CONFLICT with a spam_log_id and
captcha_site_key. On the frontend side we, the aforementioned
interceptor detects this error, opens a modal asking the user to solve
the captcha. If the captcha is solved successfully, it re-issues the
request, attaching the captchaResponse. If it isn't solved, it will
throw an `UnresolvedCaptchaError` instead. The application can choose to
gracefully handle this error, or treat it as any other axios request
error.

For now the interceptors only support PATCH, POST and PUT. Future
iterations might want to switch to using HTTP headers instead, which
would mean we could extend the approach to the other HTTP methods as
well.

In this commit we are converting the issue update captcha to use this
methodology. Before using an axios interceptor we needed to track the
state of the captcha in the application and the logic looked something
like this:

```mermaid
sequenceDiagram
    participant U as User
    participant V as Vue Application
    participant G as GitLab API
    U->>V: Save issue
    V->>G: Request
    G--xV: Response with Error and Captcha Data
    V->>U: Please solve Captcha
    U->>V: Captcha Solution
    V->>G: Resend Request with solved Captcha Data
    G-->>V: Response with Success
```

Now we are doing this:

```mermaid
sequenceDiagram
    participant U as User
    participant V as Vue Application
    participant A as Axios Interceptor
    participant G as GitLab API
    U->>V: Save issue
    V->>G: Request
    G--xA: Response with Error and Captcha Data
    A->>U: Please solve Captcha
    U->>A: Captcha Solution
    A->>G: Resend Request with solved Captcha Data
    G-->>A: Response with Success
    A-->>V: Response with Success
```

This way we have decoupled the Captcha handling from our Vue
Application. For all the Vue Application knows, it is just a request
that takes a bit longer than usual. This has the benefit that adding
captcha support to new Vue endpoints is as easy as:

```js
registerCaptchaModalInterceptor(axios);
```
Co-authored-by: default avatarLukas Eipert <leipert@gitlab.com>
parent c19699f1
......@@ -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>
......
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);
},
);
}
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;
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);
}
},
},
});
},
});
});
}
......@@ -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
......
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() {
......
......@@ -19,6 +19,7 @@ const httpStatusCodes = {
UNAUTHORIZED: 401,
FORBIDDEN: 403,
NOT_FOUND: 404,
METHOD_NOT_ALLOWED: 405,
CONFLICT: 409,
GONE: 410,
UNPROCESSABLE_ENTITY: 422,
......
......@@ -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),
......
......@@ -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,
......
......@@ -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 ""
......
......@@ -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
......
......@@ -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
......
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);
});
});
});
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('');
});
});
......@@ -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
......
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