GitLab steht wegen Wartungsarbeiten am Montag, den 10. Mai, zwischen 17:00 und 19:00 Uhr nicht zur Verfügung.

Commit 1a3b292d authored by Luke Bennett's avatar Luke Bennett Committed by Sean McGivern

Resolve "No feedback when checking on checklist if potential spam was detected"

parent 9429e8ac
...@@ -8,18 +8,7 @@ import IssuablesHelper from './helpers/issuables_helper'; ...@@ -8,18 +8,7 @@ import IssuablesHelper from './helpers/issuables_helper';
export default class Issue { export default class Issue {
constructor() { constructor() {
if ($('a.btn-close').length) { if ($('a.btn-close').length) this.initIssueBtnEventListeners();
this.taskList = new TaskList({
dataType: 'issue',
fieldName: 'description',
selector: '.detail-page-description',
onSuccess: (result) => {
document.querySelector('#task_status').innerText = result.task_status;
document.querySelector('#task_status_short').innerText = result.task_status_short;
}
});
this.initIssueBtnEventListeners();
}
Issue.$btnNewBranch = $('#new-branch'); Issue.$btnNewBranch = $('#new-branch');
Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap'); Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap');
......
...@@ -9,6 +9,7 @@ import descriptionComponent from './description.vue'; ...@@ -9,6 +9,7 @@ import descriptionComponent from './description.vue';
import editedComponent from './edited.vue'; import editedComponent from './edited.vue';
import formComponent from './form.vue'; import formComponent from './form.vue';
import '../../lib/utils/url_utility'; import '../../lib/utils/url_utility';
import RecaptchaDialogImplementor from '../../vue_shared/mixins/recaptcha_dialog_implementor';
export default { export default {
props: { props: {
...@@ -149,6 +150,11 @@ export default { ...@@ -149,6 +150,11 @@ export default {
editedComponent, editedComponent,
formComponent, formComponent,
}, },
mixins: [
RecaptchaDialogImplementor,
],
methods: { methods: {
openForm() { openForm() {
if (!this.showForm) { if (!this.showForm) {
...@@ -164,9 +170,11 @@ export default { ...@@ -164,9 +170,11 @@ export default {
closeForm() { closeForm() {
this.showForm = false; this.showForm = false;
}, },
updateIssuable() { updateIssuable() {
this.service.updateIssuable(this.store.formState) this.service.updateIssuable(this.store.formState)
.then(res => res.json()) .then(res => res.json())
.then(data => this.checkForSpam(data))
.then((data) => { .then((data) => {
if (location.pathname !== data.web_url) { if (location.pathname !== data.web_url) {
gl.utils.visitUrl(data.web_url); gl.utils.visitUrl(data.web_url);
...@@ -179,11 +187,24 @@ export default { ...@@ -179,11 +187,24 @@ export default {
this.store.updateState(data); this.store.updateState(data);
eventHub.$emit('close.form'); eventHub.$emit('close.form');
}) })
.catch(() => { .catch((error) => {
eventHub.$emit('close.form'); if (error && error.name === 'SpamError') {
window.Flash(`Error updating ${this.issuableType}`); this.openRecaptcha();
} else {
eventHub.$emit('close.form');
window.Flash(`Error updating ${this.issuableType}`);
}
}); });
}, },
closeRecaptchaDialog() {
this.store.setFormState({
updateLoading: false,
});
this.closeRecaptcha();
},
deleteIssuable() { deleteIssuable() {
this.service.deleteIssuable() this.service.deleteIssuable()
.then(res => res.json()) .then(res => res.json())
...@@ -237,9 +258,9 @@ export default { ...@@ -237,9 +258,9 @@ export default {
</script> </script>
<template> <template>
<div> <div>
<div v-if="canUpdate && showForm">
<form-component <form-component
v-if="canUpdate && showForm"
:form-state="formState" :form-state="formState"
:can-destroy="canDestroy" :can-destroy="canDestroy"
:issuable-templates="issuableTemplates" :issuable-templates="issuableTemplates"
...@@ -251,30 +272,37 @@ export default { ...@@ -251,30 +272,37 @@ export default {
:can-attach-file="canAttachFile" :can-attach-file="canAttachFile"
:enable-autocomplete="enableAutocomplete" :enable-autocomplete="enableAutocomplete"
/> />
<div v-else>
<title-component <recaptcha-dialog
:issuable-ref="issuableRef" v-show="showRecaptcha"
:can-update="canUpdate" :html="recaptchaHTML"
:title-html="state.titleHtml" @close="closeRecaptchaDialog"
:title-text="state.titleText" />
:show-inline-edit-button="showInlineEditButton" </div>
/> <div v-else>
<description-component <title-component
v-if="state.descriptionHtml" :issuable-ref="issuableRef"
:can-update="canUpdate" :can-update="canUpdate"
:description-html="state.descriptionHtml" :title-html="state.titleHtml"
:description-text="state.descriptionText" :title-text="state.titleText"
:updated-at="state.updatedAt" :show-inline-edit-button="showInlineEditButton"
:task-status="state.taskStatus" />
:issuable-type="issuableType" <description-component
:update-url="updateEndpoint" v-if="state.descriptionHtml"
/> :can-update="canUpdate"
<edited-component :description-html="state.descriptionHtml"
v-if="hasUpdated" :description-text="state.descriptionText"
:updated-at="state.updatedAt" :updated-at="state.updatedAt"
:updated-by-name="state.updatedByName" :task-status="state.taskStatus"
:updated-by-path="state.updatedByPath" :issuable-type="issuableType"
/> :update-url="updateEndpoint"
</div> />
<edited-component
v-if="hasUpdated"
:updated-at="state.updatedAt"
:updated-by-name="state.updatedByName"
:updated-by-path="state.updatedByPath"
/>
</div> </div>
</div>
</template> </template>
<script> <script>
import animateMixin from '../mixins/animate'; import animateMixin from '../mixins/animate';
import TaskList from '../../task_list'; import TaskList from '../../task_list';
import RecaptchaDialogImplementor from '../../vue_shared/mixins/recaptcha_dialog_implementor';
export default { export default {
mixins: [animateMixin], mixins: [
animateMixin,
RecaptchaDialogImplementor,
],
props: { props: {
canUpdate: { canUpdate: {
type: Boolean, type: Boolean,
...@@ -51,6 +56,7 @@ ...@@ -51,6 +56,7 @@
this.updateTaskStatusText(); this.updateTaskStatusText();
}, },
}, },
methods: { methods: {
renderGFM() { renderGFM() {
$(this.$refs['gfm-content']).renderGFM(); $(this.$refs['gfm-content']).renderGFM();
...@@ -61,9 +67,19 @@ ...@@ -61,9 +67,19 @@
dataType: this.issuableType, dataType: this.issuableType,
fieldName: 'description', fieldName: 'description',
selector: '.detail-page-description', selector: '.detail-page-description',
onSuccess: this.taskListUpdateSuccess.bind(this),
}); });
} }
}, },
taskListUpdateSuccess(data) {
try {
this.checkForSpam(data);
} catch (error) {
if (error && error.name === 'SpamError') this.openRecaptcha();
}
},
updateTaskStatusText() { updateTaskStatusText() {
const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/); const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/);
const $issuableHeader = $('.issuable-meta'); const $issuableHeader = $('.issuable-meta');
...@@ -109,5 +125,11 @@ ...@@ -109,5 +125,11 @@
:data-update-url="updateUrl" :data-update-url="updateUrl"
> >
</textarea> </textarea>
<recaptcha-dialog
v-show="showRecaptcha"
:html="recaptchaHTML"
@close="closeRecaptcha"
/>
</div> </div>
</template> </template>
...@@ -38,7 +38,8 @@ export default { ...@@ -38,7 +38,8 @@ export default {
}, },
primaryButtonLabel: { primaryButtonLabel: {
type: String, type: String,
required: true, required: false,
default: '',
}, },
submitDisabled: { submitDisabled: {
type: Boolean, type: Boolean,
...@@ -113,8 +114,9 @@ export default { ...@@ -113,8 +114,9 @@ export default {
{{ closeButtonLabel }} {{ closeButtonLabel }}
</button> </button>
<button <button
v-if="primaryButtonLabel"
type="button" type="button"
class="btn pull-right" class="btn pull-right js-primary-button"
:disabled="submitDisabled" :disabled="submitDisabled"
:class="btnKindClass" :class="btnKindClass"
@click="emitSubmit(true)"> @click="emitSubmit(true)">
......
<script>
import PopupDialog from './popup_dialog.vue';
export default {
name: 'recaptcha-dialog',
props: {
html: {
type: String,
required: false,
default: '',
},
},
data() {
return {
script: {},
scriptSrc: 'https://www.google.com/recaptcha/api.js',
};
},
components: {
PopupDialog,
},
methods: {
appendRecaptchaScript() {
this.removeRecaptchaScript();
const script = document.createElement('script');
script.src = this.scriptSrc;
script.classList.add('js-recaptcha-script');
script.async = true;
script.defer = true;
this.script = script;
document.body.appendChild(script);
},
removeRecaptchaScript() {
if (this.script instanceof Element) this.script.remove();
},
close() {
this.removeRecaptchaScript();
this.$emit('close');
},
submit() {
this.$el.querySelector('form').submit();
},
},
watch: {
html() {
this.appendRecaptchaScript();
},
},
mounted() {
window.recaptchaDialogCallback = this.submit.bind(this);
},
};
</script>
<template>
<popup-dialog
kind="warning"
class="recaptcha-dialog js-recaptcha-dialog"
:hide-footer="true"
:title="__('Please solve the reCAPTCHA')"
@toggle="close"
>
<div slot="body">
<p>
{{__('We want to be sure it is you, please confirm you are not a robot.')}}
</p>
<div
ref="recaptcha"
v-html="html"
></div>
</div>
</popup-dialog>
</template>
import RecaptchaDialog from '../components/recaptcha_dialog.vue';
export default {
data() {
return {
showRecaptcha: false,
recaptchaHTML: '',
};
},
components: {
RecaptchaDialog,
},
methods: {
openRecaptcha() {
this.showRecaptcha = true;
},
closeRecaptcha() {
this.showRecaptcha = false;
},
checkForSpam(data) {
if (!data.recaptcha_html) return data;
this.recaptchaHTML = data.recaptcha_html;
const spamError = new Error(data.error_message);
spamError.name = 'SpamError';
spamError.message = 'SpamError';
throw spamError;
},
},
};
...@@ -48,3 +48,10 @@ body.modal-open { ...@@ -48,3 +48,10 @@ body.modal-open {
display: block; display: block;
} }
.recaptcha-dialog .recaptcha-form {
display: inline-block;
.recaptcha {
margin: 0;
}
}
...@@ -25,7 +25,7 @@ def update ...@@ -25,7 +25,7 @@ def update
end end
format.json do format.json do
render_entity_json recaptcha_check_with_fallback(false) { render_entity_json }
end end
end end
......
...@@ -23,8 +23,8 @@ def ensure_spam_config_loaded! ...@@ -23,8 +23,8 @@ def ensure_spam_config_loaded!
@spam_config_loaded = Gitlab::Recaptcha.load_configurations! @spam_config_loaded = Gitlab::Recaptcha.load_configurations!
end end
def recaptcha_check_with_fallback(&fallback) def recaptcha_check_with_fallback(should_redirect = true, &fallback)
if spammable.valid? if should_redirect && spammable.valid?
redirect_to spammable_path redirect_to spammable_path
elsif render_recaptcha? elsif render_recaptcha?
ensure_spam_config_loaded! ensure_spam_config_loaded!
...@@ -33,7 +33,18 @@ def recaptcha_check_with_fallback(&fallback) ...@@ -33,7 +33,18 @@ def recaptcha_check_with_fallback(&fallback)
flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
end end
render :verify respond_to do |format|
format.html do
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)
render json: { recaptcha_html: recaptcha_html }
end
end
else else
yield yield
end end
......
- humanized_resource_name = spammable.class.model_name.human.downcase - humanized_resource_name = spammable.class.model_name.human.downcase
- resource_name = spammable.class.model_name.singular
%h3.page-title %h3.page-title
Anti-spam verification Anti-spam verification
...@@ -8,16 +7,4 @@ ...@@ -8,16 +7,4 @@
%p %p
#{"We detected potential spam in the #{humanized_resource_name}. Please solve the reCAPTCHA to proceed."} #{"We detected potential spam in the #{humanized_resource_name}. Please solve the reCAPTCHA to proceed."}
= form_for form do |f| = render 'shared/recaptcha_form', spammable: spammable
.recaptcha
- params[resource_name].each do |field, value|
= hidden_field(resource_name, field, value: value)
= hidden_field_tag(:spam_log_id, spammable.spam_log.id)
= hidden_field_tag(:recaptcha_verification, true)
= recaptcha_tags
-# Yields a block with given extra params.
= yield
.row-content-block.footer-block
= f.submit "Submit #{humanized_resource_name}", class: 'btn btn-create'
- resource_name = spammable.class.model_name.singular
- humanized_resource_name = spammable.class.model_name.human.downcase
- script = local_assigns.fetch(:script, true)
- has_submit = local_assigns.fetch(:has_submit, true)
= form_for resource_name, method: :post, html: { class: 'recaptcha-form js-recaptcha-form' } do |f|
.recaptcha
- params[resource_name].each do |field, value|
= hidden_field(resource_name, field, value: value)
= hidden_field_tag(:spam_log_id, spammable.spam_log.id)
= hidden_field_tag(:recaptcha_verification, true)
= recaptcha_tags script: script, callback: 'recaptchaDialogCallback'
-# Yields a block with given extra params.
= yield
- if has_submit
.row-content-block.footer-block
= f.submit "Submit #{humanized_resource_name}", class: 'btn btn-create'
---
title: Add recaptcha modal to issue updates detected as spam
merge_request: 15408
author:
type: fixed
...@@ -272,6 +272,20 @@ def move_issue ...@@ -272,6 +272,20 @@ def move_issue
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
expect(issue.reload.title).to eq('New title') expect(issue.reload.title).to eq('New title')
end end
context 'when Akismet is enabled and the issue is identified as spam' do
before do
stub_application_setting(recaptcha_enabled: true)
allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
end
it 'renders json with recaptcha_html' do
subject
expect(JSON.parse(response.body)).to have_key('recaptcha_html')
end
end
end end
context 'when user does not have access to update issue' do context 'when user does not have access to update issue' do
...@@ -504,17 +518,16 @@ def go(id:) ...@@ -504,17 +518,16 @@ def go(id:)
expect(spam_logs.first.recaptcha_verified).to be_falsey expect(spam_logs.first.recaptcha_verified).to be_falsey
end end
it 'renders json errors' do it 'renders recaptcha_html json response' do
update_issue update_issue
expect(json_response) expect(json_response).to have_key('recaptcha_html')
.to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."])
end end
it 'returns 422 status' do it 'returns 200 status' do
update_issue update_issue
expect(response).to have_gitlab_http_status(422) expect(response).to have_gitlab_http_status(200)
end end
end end
......