search

magento / security-package

Magento Security Extensions
Open Software License 3.0
73 stars 69 forks source link

Allows Global ReCAPTCHA #288 #289

Closed thomas-kl1 closed 3 years ago

thomas-kl1 commented 3 years ago

Add the alternative domain https://www.recaptcha.net/recaptcha/api.js

Description (*)

As a merchant I want to select the captcha domain:

https://www.google.com/recaptcha/api.js https://www.recaptcha.net/recaptcha/api.js See Google Documentation: https://developers.google.com/recaptcha/docs/faq#can-i-use-recaptcha-globally

Fixed Issues (if relevant)

  1. magento/security-package#288: Allow use reCAPTCHA gloabally

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

Beside that the PR aims to fix components dependencies declaration. This PR also add return type hint when possible to method signatures.

Contribution checklist (*)

thomas-kl1 commented 3 years ago

@magento run all tests

thomas-kl1 commented 3 years ago

hi @nathanjosiah could you take a look on my comments?

thomas-kl1 commented 3 years ago

poke @nathanjosiah

nathanjosiah commented 3 years ago

@thomas-kl1 Apologies for the delay, I am on paternity leave but I left a comment and resolved some existing ones.

thomas-kl1 commented 3 years ago

@nathanjosiah ho! Congrats! :confetti_ball: :confetti_ball:

thomas-kl1 commented 3 years ago

@magento run all tests

thomas-kl1 commented 3 years ago

The failures in tests does not seems to be related to this PR.

nathanjosiah commented 3 years ago

Connected to #288

nathanjosiah commented 3 years ago

@thomas-kl1 can you merge develop and rerun builds?

thomas-kl1 commented 3 years ago

@magento run all tests

magento-automated-testing[bot] commented 3 years ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

nathanjosiah commented 3 years ago

@thomas-kl1 Something appears to be broken with MFTF test generation. The error log indicates it is unable to generate at least one of the tests. Can you check that you are able to build locally?

thomas-kl1 commented 3 years ago

Hi @nathanjosiah Would be easier for me if you could actually give the result of mftf.log, it's really hard to run test locally with the complete suite..

magento-automated-testing[bot] commented 3 years ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

nathanjosiah commented 3 years ago

@magento run Functional Tests CE against 2.4.3-develop

magento-automated-testing[bot] commented 3 years ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

nathanjosiah commented 3 years ago

@thomas-kl1 Looks like there are some errors in certain scenarios in the latest build (see https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/security-package/pull/289/bf46e4fe54b374d1b9fcff72e9c97fce/Functional/allure-report-ce/index.html#categories/bda2d1d522cd5c6d3b119ea9eba8bd02/6f6a43bc714a93f9/) specifically

Magento_ReCaptchaFrontendUi/js/reCaptcha.js 53:68 Uncaught TypeError: Cannot read property 'api_url' of undefined
nathanjosiah commented 3 years ago

@thomas-kl1 Pinging again for visibility.

thomas-kl1 commented 3 years ago

Hi @nathanjosiah I'll rebase and take a look when I'll have some free time, sorry for the delay

nathanjosiah commented 3 years ago

No worries. I wanted to make sure you saw my comment and that you were still wanting to work on this issue

nathanjosiah commented 3 years ago

Closing due to inactivity.

thomas-kl1 commented 3 years ago

Rly? Make it draft but don't close it