Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

security-package: Added try/catch to Magento\ReCaptchaUi\Model\RequesttHandler #175

Merged
merged 8 commits into from
Apr 7, 2020
Merged

security-package: Added try/catch to Magento\ReCaptchaUi\Model\RequesttHandler #175

merged 8 commits into from
Apr 7, 2020

Conversation

engcom-Kilo
Copy link
Contributor

@engcom-Kilo engcom-Kilo commented Apr 1, 2020 •
edited by engcom-Delta
Loading

Description (*)

Added try/catch construction to RequesttHandler class for to capture exceptions.

Fixed Issues (if relevant)

#197

Manual testing scenarios (*)

  1. Enable reCaptcha v2 (I an not Robot) for Admin Forgot Password
  2. Add to reCaptcha v2 (I an not Robot) settings API keys test/test
  3. Enable reCaptcha v2 (I an not Robot) for Admin Forgot Password
  4. Open the page Admin Forgot Password (host/admin/admin/auth/forgotpassword/)
  5. Fill form and try to send
  6. Will returned fatal error, with error message report

There_has_been_an_error_processing_your_request

Questions or comments

Contribution checklist (*)

  • Author has signed the Adobe CLA
  • 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)

@engcom-Kilo engcom-Kilo added Component: Google reCAPTCHA Issues and Pull Requests related to reCAPTCHA should be marked with this label Component: Admin labels Apr 1, 2020
@engcom-Kilo engcom-Kilo requested a review from naydav April 1, 2020 14:41
@engcom-Delta engcom-Delta self-assigned this Apr 2, 2020
@engcom-Delta
Copy link

@engcom-Kilo issue with exception is fixed, but seems like we have redurant error message Please enter an email address. if email and magento CAPTCHA were entered correctly:

image

@engcom-Foxtrot engcom-Foxtrot self-assigned this Apr 3, 2020
@engcom-Foxtrot engcom-Foxtrot force-pushed the security-package/try-catch-for-handler branch from 6c5de1a to b9b151d Compare April 3, 2020 10:38
@engcom-Foxtrot engcom-Foxtrot force-pushed the security-package/try-catch-for-handler branch from b9b151d to bd53306 Compare April 3, 2020 11:43
This was referenced Apr 3, 2020
Closed
Closed
@engcom-Delta
Copy link

engcom-Delta commented Apr 3, 2020 •
edited
Loading

@engcom-Kilo @engcom-Foxtrot @naydav
Generally, PR fix initial issue with exception error.
There is some cases that need to be approved as expected behavior or need to be fixed
Case 1: Wrong API keys for reCAPTCHA v2 ("I am not a robot")
Result when try submit form: Additional error message "Please enter an email address"
image

image

image

Case 2: Wrong API keys for reCAPTCHA v2-v3 Invisible
Result when try submit form: Error in console, form is not submitting
image

image

@engcom-Delta
Copy link

✔️ Initial issue is fixed
Testing results:
Case 1: Wrong API keys for reCAPTCHA v2 ("I am not a robot")
Result when try submit form: Additional error message "Please enter an email address"
image

image

image

Case 2: Wrong API keys for reCAPTCHA v2-v3 Invisible
Result when try submit form: Error in console, form is not submitting
image

image

Case 3 [:heavy_check_mark: issue #197 is fixed]: reCAPTCHA v2 ("I am not a robot") is unchecked or reCAPTCHA v2 ("I am not a robot") API Secret Key is incorrect
Result:
#175PrPayPal

Case 4: reCAPTCHA v2-v3 Invisible API Secret Key is incorrect
Result:
#175Paypalv2v3

@naydav
Copy link
Contributor

naydav commented Apr 7, 2020 •
edited
Loading

Case 2: Wrong API keys for reCAPTCHA v2-v3 Invisible
It's should be investigated and fixed in #191
The PR contains JS refactoring.

Case 4: reCAPTCHA v2-v3 Invisible API Secret Key is incorrect
Should be extracted in a separate ticket. The problem is related to incorrect validation message.

@engcom-Delta thank you for QA

@naydav naydav merged commit 12e8fd2 into 1.0-develop Apr 7, 2020
@naydav naydav mentioned this pull request Apr 7, 2020
Closed
@sdzhepa sdzhepa mentioned this pull request Apr 9, 2020
Closed
9 tasks
@lenaorobei lenaorobei deleted the security-package/try-catch-for-handler branch May 7, 2020 16:38
magento-devops-reposync-svc pushed a commit that referenced this pull request Feb 11, 2025
@pawan-adobe-security
Merge pull request #175 from magento-cia/cia-2.4.8-beta2-develop-bugf…
442bb83 
…ix-101272024

AC-10142: Wishlist sharing email issue fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@engcom-Foxtrot engcom-Foxtrot Awaiting requested review from engcom-Foxtrot

@naydav naydav Awaiting requested review from naydav

Assignees

@engcom-Delta engcom-Delta

@engcom-Foxtrot engcom-Foxtrot

Labels
Component: Admin Component: Google reCAPTCHA Issues and Pull Requests related to reCAPTCHA should be marked with this label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@engcom-Kilo @engcom-Delta @naydav @engcom-Foxtrot