New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 828431 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Block bad certs and mixed content in PaymentRequestEvent.openWindow()

Project Member Reported by rouslan@chromium.org, Apr 3 2018

Issue description

Block bad certs and mixed content in PaymentRequestEvent.openWindow()
 
Status: Started (was: Assigned)
Is WebContentsDelegate::ShouldAllowRunningInsecureContent() the best option for blocking mixed content?
Also need to verify that the payment handler is https:// only.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99a348cf6378c095c86cb6dfc717b0453ab9e222

commit 99a348cf6378c095c86cb6dfc717b0453ab9e222
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Mon Apr 16 16:15:09 2018

[PH][Desktop] Block unsecure pages.

Before this patch, a payment handler page could be unsecure.

This patch checks for the following conditions:
- Unsecure origin.
- Non-cryptographic scheme.
- Invalid certificate.
- Flagged in safe browsing database.
If any of these conditions are hit, Chrome aborts payment by closing the
payment handler page and showing an error message.

After this patch, if a payment handler page is detected to be unsecure,
Chrome closes the payment handler page and shows an error message.

Bug:  828431 
Change-Id: Ifdc5a3e3ebf9c511f21aa03dee2d8d3230ac8b88
Reviewed-on: https://chromium-review.googlesource.com/1012451
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550987}
[modify] https://crrev.com/99a348cf6378c095c86cb6dfc717b0453ab9e222/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/99a348cf6378c095c86cb6dfc717b0453ab9e222/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc
[modify] https://crrev.com/99a348cf6378c095c86cb6dfc717b0453ab9e222/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.h

Labels: Needs-Feedback
rouslan@ Request you to provide a sample file/URL and the steps to reproduce the issue to verify the fix on the latest M-68 chrome build.

Thanks..
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99a348cf6378c095c86cb6dfc717b0453ab9e222

commit 99a348cf6378c095c86cb6dfc717b0453ab9e222
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Mon Apr 16 16:15:09 2018

[PH][Desktop] Block unsecure pages.

Before this patch, a payment handler page could be unsecure.

This patch checks for the following conditions:
- Unsecure origin.
- Non-cryptographic scheme.
- Invalid certificate.
- Flagged in safe browsing database.
If any of these conditions are hit, Chrome aborts payment by closing the
payment handler page and showing an error message.

After this patch, if a payment handler page is detected to be unsecure,
Chrome closes the payment handler page and shows an error message.

Bug:  828431 
Change-Id: Ifdc5a3e3ebf9c511f21aa03dee2d8d3230ac8b88
Reviewed-on: https://chromium-review.googlesource.com/1012451
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550987}
[modify] https://crrev.com/99a348cf6378c095c86cb6dfc717b0453ab9e222/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/99a348cf6378c095c86cb6dfc717b0453ab9e222/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc
[modify] https://crrev.com/99a348cf6378c095c86cb6dfc717b0453ab9e222/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.h

Project Member

Comment 6 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f5e3fa14785ec3f721da24a2b441ebb05300013e

commit f5e3fa14785ec3f721da24a2b441ebb05300013e
Author: gogerald <gogerald@google.com>
Date: Fri May 04 13:50:34 2018

[PH][Android] Block unsecure pages

This CL also enables localhost on Desktop for debug.

This is the counter part of the desktop CL,
https://chromium-review.googlesource.com/c/chromium/src/+/1012451

Test:
1), go to 'https://rsolomakhin.github.io/pr/bob/'
2), click 'buy' button
3), choose 'personal bobpay' from 'https://gogerald.github.io'
4), click 'PAY'
5), login if necessary (any more than five characters name and passowrd).
6), click 'Navigate to http page'
7), the window should close, the payment request dialog is shown to allow users
choose different payment method.

Bug:  828431 
Change-Id: I0bbe5e97d364f7cd172ae7cfc43962dd314398e2
Reviewed-on: https://chromium-review.googlesource.com/1031056
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556040}
[modify] https://crrev.com/f5e3fa14785ec3f721da24a2b441ebb05300013e/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[modify] https://crrev.com/f5e3fa14785ec3f721da24a2b441ebb05300013e/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java
[modify] https://crrev.com/f5e3fa14785ec3f721da24a2b441ebb05300013e/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6fcf49e3f5f49ac0b84f69b2a9833e5a62370cdc

commit 6fcf49e3f5f49ac0b84f69b2a9833e5a62370cdc
Author: gogerald <gogerald@google.com>
Date: Mon May 14 13:26:19 2018

[Payment] Abort payment request when opened window has mixed content

Test:
1, open 'https://rsolomakhin.github.io/pr/bob/'
2, clock 'Buy'
3, choose 'Just-in-time Bobpay'
4, click 'Pay'
5, click 'Navigate to mixed content page', which opens 'https://mixed.badssl.com'
6, Payment request should abort.

Bug:  828431 
Change-Id: I38a1725b6265b6450b0cfa88d8234eed2949bb2c
Reviewed-on: https://chromium-review.googlesource.com/1056012
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558280}
[modify] https://crrev.com/6fcf49e3f5f49ac0b84f69b2a9833e5a62370cdc/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc
[modify] https://crrev.com/6fcf49e3f5f49ac0b84f69b2a9833e5a62370cdc/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.h

Comment 8 by ma...@chromium.org, May 30 2018

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b50acaaf5e38cc4531cc980d79a17111ccec4c06

commit b50acaaf5e38cc4531cc980d79a17111ccec4c06
Author: gogerald <gogerald@google.com>
Date: Mon Jun 11 21:53:10 2018

[Payments] Allow 'about:blank' navigation and ignore same document navigation

Bug:  828431 
Change-Id: Ie258060d0804137e9bce1d0e2d44eb5ed0a74fc3
Reviewed-on: https://chromium-review.googlesource.com/1095561
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566161}
[modify] https://crrev.com/b50acaaf5e38cc4531cc980d79a17111ccec4c06/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc

Labels: M-68 Merge-Request-68
Project Member

Comment 11 by sheriffbot@chromium.org, Jun 12 2018

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Has this already been verified in canary? 
Cc: ma...@chromium.org
Not yet, will update here when it has been verified,
FYI, this has been verified in canary,
Labels: -Merge-Review-68 Merge-Approved-68
Approved - branch:3440
Cc: abdulsyed@chromium.org
Pls merge you change to M68 branch 3440 ASAP so we can pick it up for this week Beta release. Merge has to happen latest by 1:00 PM PT tomorrow, Tuesday (06/19), so we can pick it up for Wednesday Beta release.




Project Member

Comment 18 by sheriffbot@chromium.org, Jun 19 2018

Cc: abdulsyed@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 19 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20cf87bbd185cd3473b604e665aa85c830dadcc7

commit 20cf87bbd185cd3473b604e665aa85c830dadcc7
Author: gogerald <gogerald@google.com>
Date: Tue Jun 19 16:34:49 2018

[Payments] Allow 'about:blank' navigation and ignore same document navigation

Bug:  828431 
Change-Id: Ie258060d0804137e9bce1d0e2d44eb5ed0a74fc3
Reviewed-on: https://chromium-review.googlesource.com/1095561
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#566161}(cherry picked from commit b50acaaf5e38cc4531cc980d79a17111ccec4c06)
Reviewed-on: https://chromium-review.googlesource.com/1106437
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#445}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/20cf87bbd185cd3473b604e665aa85c830dadcc7/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc

Unable to verify this issue on reported version and latest beta 68.0.3440.33 as we are seeing error "NotSupportedError: The payment methods "https://emerald-eon.appspot.com/bobpay", "interledger" are not supported
" on navigating to "https://rsolomakhin.github.io/pr/bob/" [Checked with steps present in comment#7]and clicking on buy. Attaching screenshot for reference.

@rouslan: Please help in verifying the fix as we are unable to proceed from step-2[comment#7].

Thanks!
828431_error.png
71.6 KB View Download

Comment 22 by ma...@chromium.org, Jun 20 2018

Owner: gogerald@chromium.org
We moved our test page, please refer updated test plan, https://docs.google.com/document/d/1MsJOqJ0rI4emNzYg6g5M2XiNTYFQVyprpGXTlhEizlY/edit?ts=5b19fd5f#heading=h.pgo89jvk1hjt

'https://rsolomakhin.github.io/pr/bob/' now only works with my private debug test bobpay, which might be broken without notice for debug.

gogerald@, i do not see any difference with the updated test plan (per c#23) as well and seeing the same behavior as of c#21. Can you please double check?

Thank you!
It works in my test. Could you post your test steps?
Project Member

Comment 26 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b6d9fdf1a13ec350c2f8235ef5961642f7e6fd0

commit 1b6d9fdf1a13ec350c2f8235ef5961642f7e6fd0
Author: gogerald <gogerald@google.com>
Date: Tue Jul 10 13:49:57 2018

[Payments] Abort payment when interstitial page attached

This is the missing counterpart changes as on Desktop,
PaymentHandlerWebFlowViewController::DidAttachInterstitialPage

Bug:  828431 
Change-Id: I38a0950dfa2aef1895902baccb0df83492064f42
Reviewed-on: https://chromium-review.googlesource.com/1129213
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573709}
[modify] https://crrev.com/1b6d9fdf1a13ec350c2f8235ef5961642f7e6fd0/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java

Sign in to add a comment