Skip to content

Draft: Convert issue creation to use new CAPTCHA modal

Chad Woolley requested to merge caw-convert-issue-creation-to-new-captcha-modal into master

UPDATE - Closed without further action

UPDATE:

  • The approach which was planned in this MR (to make an Javascript-based interceptor for the HTML form submission) DOES NOT seem to be possible (or at least would be prohibitively complex). See details in this comment
  • So, after discussing this further with @pslaughter , the best option seems to be keeping this existing alternative HTML-form-submission CAPTCHA support logic, but cleaning it up and making it very clear that it should only be used for HTML-only forms, and the Apollo/Axios interceptors should be preferred for any JS/JSON form submissions.

Reasons:

  1. This is the simplest way to achieve the main goal, which is to eliminate this as a confusing source of tech debt. We won't eliminate it, but we will make it less confusing.
  2. This also meets the goal of being able to quickly and easily add CAPTCHA support for additional HTML-only form submissions in other areas of the app.

@cwoolley-gitlab will proceed with this approach on a new MR (TODO: add link), and close this one out.

What does this MR do?

This MR converts issue creation to use new CAPTCHA modal. Issue update has already been converted.

Issue creation is the only remaining place that uses the old HAML-based reCAPTCHA-specific modal implementation.

Existing Implementation

We want to remove the following existing implementation:

  • app/views/projects/issues/verify.html.haml
    • TODO: What about the hidden fields merge_request_to_resolve_discussions_of and discussions_to_resolve? Do these need to be preserved somehow?
  • app/views/layouts/_recaptcha_verification.html.haml
  • app/views/shared/_recaptcha_form.html.haml

We can also remove:

  • The #extract_legacy_spam_params_to_headers method from app/controllers/concerns/spammable_actions.rb
  • The respond_to html block in #recaptcha_check_with_fallback from app/controllers/concerns/spammable_actions.rb (this is where the "verify" template above is rendered).

Proposed Implementation

We will replace the existing implemenation with the new CAPTCHA support, which is found under app/assets/javascripts/captcha.

  • We want to avoid having to convert all of the issue creation logic from HAML to Vue app(s).
  • But we want to still reuse the new CAPTCHA modal infrastructure to present the CAPTCHA:
    • app/assets/javascripts/captcha/captcha_modal.vue
    • app/assets/javascripts/captcha/wait_for_captcha_to_be_solved.js
  • So, we will use a Vue renderless component to intercept the existing form submission process and display the modal.
  • app/assets/javascripts/vue_shared/components/local_storage_sync.vue is an example of a Vue renderless component.
  • Here are some other existing examples of Vue renderless components (every Vue component which does not have the string template in it):
$ grep -rL "template" app/assets/javascripts/**/*.vue
app/assets/javascripts/behaviors/shortcuts/shortcut.vue
app/assets/javascripts/experimentation/components/gitlab_experiment.vue
app/assets/javascripts/feature_flags/components/strategies/default.vue
app/assets/javascripts/jobs/components/log/line.vue
app/assets/javascripts/jobs/components/log/line_number.vue
app/assets/javascripts/members/components/table/members_table_cell.vue
app/assets/javascripts/notes/components/discussion_navigator.vue
app/assets/javascripts/notes/components/discussion_notes_replies_wrapper.vue
app/assets/javascripts/notes/components/sidebar_subscription.vue
app/assets/javascripts/pipeline_editor/components/ui/confirm_unsaved_changes_dialog.vue
app/assets/javascripts/registry/explorer/components/delete_image.vue
app/assets/javascripts/sidebar/components/assignees/assignees_realtime.vue
app/assets/javascripts/static_site_editor/components/unsaved_changes_confirm_dialog.vue
app/assets/javascripts/vue_shared/components/gfm_autocomplete/gfm_autocomplete.vue
app/assets/javascripts/vue_shared/components/local_storage_sync.vue
app/assets/javascripts/vue_shared/components/ordered_layout.vue
app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/label_item.vue
app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/label_item.vue
app/assets/javascripts/vue_shared/components/url_sync.vue

However, at first glance, none of these seem to do the type of form submission interception we need.

So, what we will need is:

(NOTE: tentative, work in progress)

  1. Create a new Vue component
  2. Mount it when the Issue "new" form is rendered (app/views/projects/issues/new.html.haml
  3. In the mounted() lifecycle event of the Vue component, attach a listener to the form submission event.
  4. Write logic in the Vue component to proxy the form submission request
  5. The proxy logic will handle automatically displaying the CAPTCHA form and re-submitting the original request if the CAPTCHA headers exist.
  6. This intercept and re-submit logic should be similar in concept to what we already do in the case of Apollo and Axios:
    • app/assets/javascripts/captcha/apollo_captcha_link.js
    • app/assets/javascripts/captcha/captcha_modal_axios_interceptor.js

Additional requirements

Screenshots or Screencasts (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

See the testing section of the CAPTCHA epic for detailed instructions on how to test spam/CAPTCHA in a dev environment.

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related

Edited by Chad Woolley

Merge request reports