Implement spam mechanism in snippets VUE edit refactor
This page may contain information related to upcoming products, features and functionality. It is important to note that the information presented is for informational purposes only, so please do not rely on the information for purchasing or planning purposes. Just like with all projects, the items mentioned on the page are subject to change or delay, and the development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc.
Why are we doing this work
- We refactored snippets to be implemented in Vue, but support for reCAPTCHA was not added in the initial iteration. This issue will add the missing reCAPTCHA support to snippet creation and editing.
- It will also re-implement the reCAPTCHA support in a modern way which is compliant with current GitLab engineering best practices on the backend and frontend:
- The existing reCAPTCHA approach uses several outdated/anti/non-standard patterns: It is rendered on the backend via the recaptcha ruby gem + HAML which embeds the original changes, and the resulting HTML sent in a JSON response field to be displayed and submitted via a deprecated Vue modal.
- The UI will be implemented in native Vue.
- It will be GraphQL-first, with no REST APIs involved, and only the necessary scalar fields being sent via graphQL (instead of the rendered HTML being sent in a JSON field)
- It will use the standard Pajamas/gitlab-ui Vue Modal component, rather than the deprecated modal.
- When this modern reCAPTCHA implementation is completed, we will positioned to be able to replace much the old implementation from the codebase (which is only used by issues), as well as to easily use this new implementation in any future areas of the product which need to have reCAPTCHA support added.
- It will use the native javascript reCAPTCHA API and callbacks integrated with the Vue component, rather than the standard HTML form submission approach.
Relevant links
- Original Description
- Exploration and documentation of existing reCAPTCHA support and initial plans for new architecture (note that some of these plans have changed from their initial form):
- Initial MR exploring and discussing the proposed implementation with a fully-functional spike/prototype/Proof of Concept, to validate the architecture before breaking it down into separate iterative implementation MRs: Snippets GraphQL client-side spam/captcha spike
Non-functional requirements
-
Documentation: -
Update relevant docs to reflect new approach.
-
-
Feature flag: snippet_spam
-
Performance: -
Testing:
Implementation plan
NOTE: Numbering indicates implementation order and rebase hierarchy of unmerged planned/in-progress MRs.
-
Fully functional spike/prototype/proof of concept: Snippets GraphQL client-side spam/captcha spike. - NOTE: The subsequent MRs will be smaller, iterative, better-tested parts of the functionality in this MR.
- Backend:
-
1. (MERGED) MR: Remove unnecessary recaptcha verification param and unused snippets verify views - BRANCH: remove-recaptcha-verification-param
-
2. (MERGED - but needs followup MRs) Refactor SpamActionService and its usage - BRANCH: refactor-spam-action-service
- Perform the preliminary refactoring of
spam_action_service.rb
as suggested in this section of the docs. See more discussion here
- Perform the preliminary refactoring of
- [-]
X. MR: Refactor common reCAPTCHA logic out of spammable_actions.rb to be usable from service layer - BRANCH:UPDATE - this MR is not be needed, almost all of its intended changes are no longer required after Refactor SpamActionService and its usagerefactor-out-common-recaptcha-logic
-
3. (MERGED) MR: Refactor spam and recaptcha-related mutations and services to better support future reCAPTCHA support - BRANCH: refactor-spammable-recaptcha-mutations
- Add GraphQL fields necessary to support reCAPTCHA:
-
- Frontend:
-
1. (MERGED) MR: Introduce alternative recaptcha script initialization (not dependent upon any backend work, can be done in parallel) - BRANCH: refactor-recaptcha-script-initialization
-
5. (MERGED) MR: Implement new reCAPTCHA modal - BRANCH: new-recaptcha-modal
-
6. (MERGED) ISSUE: Submit Snippet after reCAPTCHA cancel causes console.error MR -Refactor snippets edit spec and small fixes - Address Apollo-related bug with snippets mixin undefined blob during captcha flow
-
- Final:
-
Enable snippet spam feature flag: #262013 (closed)
-
Future follow-on Issues/MRs
UPDATE: All of the following task list has been copied verbatim to this section in the CAPTCHA epic description description, so that this otherwise-completed issue can be closed.
-
(Issue created) [Feature flag] Enable snippet_spam flag -
Add more robust error handling to reCAPTCHA modal. Currently it only handles a single simple failure case. -
Refactor mutationTypes
inspec/frontend/snippets/components/edit_spec.js
to make them all factories. See !52044 (comment 493848680) -
MR: Refactor snippets edit spec and small fixes -
Review all comment threads on !51840 (merged) and open follow-up issues as appropriate -
Look into DRYing up duplication in app/graphql/mutations/snippets/create.rb
andapp/graphql/mutations/snippets/update.rb
-
(Issue created) Cleanup callback set in init_recaptcha_script -
In the GraphQL API, consider making captchaSiteKey
,needsCaptchaResponse
, andspamLogId
to be their own cohesivetype
. -
Add any missing feature/integration coverage of spam workflows for issues and snippets (there is already some, not sure if we need more) -
Convert issues GraphQL endpoints to have spam/reCAPTCHA protection through new framework -
Convert issues spam/recaptcha Vue components and backend support to new framework and delete old support, as much as possible given that it's still using the REST API. -
Add spam/recaptcha protection for issues created/updated via GraphQL API -
Investigate FriendlyCaptcha Support (and or other captcha implementations - TODO: link other issues) - The changes made as part of this issue intentionally took FriendlyCaptcha support into account, and I believe we are in a good place to attempt implementing it (possibly after some of the other above issues are completed too). -
Refactor terminology in DB and models of render_recaptcha
toneeds_captcha_response
, to better reflect the new implementation and not be coupled to any specific CAPTCHA implementation or approach. See this thread for more details.
Testing Notes
See the testing notes section on the epic for details on how to test CAPTCHA locally: https://gitlab.com/groups/gitlab-org/-/epics/5527#testing-notes
Original Description
Click to expand Original Description
When a snippet is created or updated, one of the workflows we have in place is to check whether it is spam or not. These checkings are done in the snippet controllers. Since we're moving away from these controllers and use the VUE refactor + GraphQL mutations, we need to implement this workflow as well in VUE.
The checking to render whether we should render or not the captcha is done in recaptcha_check_with_fallback. Besides, we pass some info related to a possible former captcha request to the services that create/update snippets. This is done in https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/snippets_controller.rb#L52 and https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/snippets_controller.rb#L66, where we include spammable_params
inside the params sent by the user.
Then, in recaptcha_check_with_fallback
, based on those params and the snippet status (whether it is valid or not), we opt for redirecting the user to the snippet show page, render the captcha page or render the edit page showing some validation errors.
We have to dig into this more but I suppose, at least from the BE perspective, that we need to update the arguments the snippet mutations accept in GraphQL and also update the attributes returned by those mutations to include the spam attributes in the snippet type (need_recaptcha
, spam
, and maybe others).
From the FE perspective, we might need to create a new captcha component or update it if it exists.
This issue is really important because we cannot globally enable the edit VUE refactor without this working.
Testing Activity
- Adapt or clone existing specs for the snippet GraphQL endpoints to check the spam mechanism is still in place.
- FE need to add some specs to check that it passess the proper attributes to the GraphQL endpoint
@vij: *Add existing captcha tests to cover the GraphQL mutations
DON'T FORGET: There's a preexisting feature spec we can work with. !43868 (comment 422321741)
MR Breakdown
-
We need to update the endpoint to receive the param
api
,recaptcha_verified
, andspam_log_id
. Then, we need to pass them to the snippets create and update services along with the request in place. These services wil handle automatically the spam once they receive these attributes. -
FE need to pass these attributes from the snippet edit page in VUE
-
1 MR to update the snippets create mutation to add the necessary params and pass them to the create service.
-
1 MR to update the snippets update mutation to add the necessary params and pass them to the update service.
-
1 MR FE to update the edit page in VUE in order to add the necessary componentes
@vij:
- Update Snippet GraphQL mutations (create, update) to accept the captcha args, may need additional work depending on FE requirements (hence weight 2)