Skip to content

Decouple reCAPTCHA type modules from the GraphQL config interface - #348

Open
lbajsarowicz wants to merge 1 commit into
magento:developfrom
lbajsarowicz:fix/41558-recaptcha-config-interface
Open

lbajsarowicz wants to merge 1 commit into
magento:developfrom
lbajsarowicz:fix/41558-recaptcha-config-interface

Conversation

@lbajsarowicz

Copy link
Copy Markdown

Description

The three reCAPTCHA type modules (Magento_ReCaptchaVersion2Checkbox, Magento_ReCaptchaVersion2Invisible, Magento_ReCaptchaVersion3Invisible) implement Magento\ReCaptchaWebapiGraphQl\Model\Adapter\ReCaptchaConfigInterface in their Model\Config classes and require magento/module-re-captcha-webapi-graph-ql in composer.json. The interface is a plain configuration contract (getWebsiteKey, getTheme, getLanguageCode, getMinimumScore, getBadgePosition) with nothing GraphQL-specific in it, yet it lives in the GraphQL module. The GraphQL module in turn requires module-re-captcha-version-3-invisible, so the two depend on each other. A store that disables or replaces the GraphQL packages (a common hardening step on Luma / Hyvä / REST-only builds) loses every reCAPTCHA type at class load time.

This PR moves the contract to Magento\ReCaptchaUi\Model\ReCaptchaConfigInterface (the three type modules already require magento/module-re-captcha-ui), keeps the GraphQL interface as a deprecated empty extension of the new one so existing implementations still satisfy it, switches the three Config classes to the new interface, and drops the GraphQL requirement from their composer.json. ReCaptchaWebapiGraphQl now declares the two V2 packages it already wires in etc/graphql/di.xml, so the dependency direction is GraphQL → reCAPTCHA types, never the reverse.

Behaviour of the GraphQL resolvers is unchanged: ReCaptchaFormConfig receives the same Config objects through DI, and ReCaptchaV3 type-hints the concrete V3 Config. The one visible change for extensions: the core Config classes no longer implement the old GraphQL interface, so an extension that type-hints Magento\ReCaptchaWebapiGraphQl\Model\Adapter\ReCaptchaConfigInterface and receives a core Config must switch to the ReCaptchaUi interface. Custom implementations of the old interface keep working because it extends the new one.

Related Pull Requests / Issues

Part of magento/magento2#41558 (non-GraphQL modules coupled to *GraphQl modules). A second PR in this repository removes the GraphQL requirements from ReCaptchaContact and ReCaptchaResendConfirmationEmail.

Manual testing scenarios

  1. Install with magento/module-re-captcha-webapi-graph-ql removed through replace in the root composer.json, enable reCAPTCHA v3 invisible for the customer login form, open the storefront login page: the badge renders and a login with a valid token succeeds.
  2. Same with reCAPTCHA v2 checkbox and v2 invisible.
  3. Standard install with GraphQL present: run the recaptchaFormConfig and recaptchaV3Config queries; output is identical to develop.

Verification

PHPCS (Magento2 standard) on the changed PHP files: 0 errors, 0 warnings. check_copyright_pr.php as run by pr-quality-gates: all files pass. No unit tests cover these classes in the repository.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Move the reCAPTCHA configuration contract to Magento_ReCaptchaUi so the
version 2 and version 3 modules no longer require Magento_ReCaptchaWebapiGraphQl.
The GraphQL interface stays as a deprecated extension of the new one; the
GraphQL module now declares the V2 packages it already wires. magento/magento2#41558
@lbajsarowicz

Copy link
Copy Markdown
Author

@magento run all tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant