Resolve "Project bots can get CAPTCHA even on a private project"

What does this MR do?

Fixes issue that project bots should not get CAPTCHAs when they otherwise should not (for example, on private projects).

See discussion on issue for more context.

Root cause

• Issues created by a "bot" in an EE instance will always receive a CAPTCHA if the spam checking is enabled and the issue is flagged as potential spam (ee/app/models/ee/issue.rb), based on the content (currently, the values of title and content).
• This will happen regardless of whether the project is considered not "publicly visible" due to being private, confidential, or has external authorization enabled
• This "bot check" behavior dates back to this commit four years ago, which was committed as part of this issue.
• However, at that time, the behavior only applied to "support bot" users
• Then, in this MR, the check was restricted to only bots and only spammable fields (currently title, description, and confidential flag).
• With the introduction of "Project bot users" in 13.0, this seems to have inadvertently changed the scope of what is a "bot" is.
• This results in "project bot" users getting CAPTCHAs when they otherwise shouldn't (because the project is private, confidential, or has external authorization enabled).

Additional context on bots and EE-specific override

Note that Project bots are not EE-specific, and also that support bots are now not EE-specific either as of 13.2

Therefore, in addition to being fixed, this logic should also be moved up to the base app/models/issue.rb class (and we can also remove the duplication around which fields to check).

Fix

Therefore, the new logic will live only on the CE app/models/issues.rb, with no EE override, and will look like this:

  def check_for_spam?
    # content created via support bots is always checked for spam, EVEN if it
    # the issue is not publicly visible
    return true if author.support_bot? && spammable_attribute_changed?

    # Only check for spam on issues which are publicly visible (and thus indexed in search engines)
    return false unless publicly_visible?

    # Only check for spam if certain attributes have changed
    spammable_attribute_changed?
  end

Does this MR meet the acceptance criteria?

Conformity

• I have included changelog trailers, or none are needed. (Does this MR need a changelog?)
• I have added/updated documentation, or it's not needed. (Is documentation required?)
• I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?)
• I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?)
• I have self-reviewed this MR per code review guidelines.
• This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines)
• I have followed the style guides.
• This change is backwards compatible across updates, or this does not apply.

Availability and Testing

See the following section of the CAPTCHA epic for notes on how to test CAPTCHA locally: https://gitlab.com/groups/gitlab-org/-/epics/5527#testing-notes

• I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.)
• I have tested this MR in all supported browsers, or it's not needed.
• I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.

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 to #16240 (closed)