Commit 3b6866da authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot

[PaymentHandler] Align security check with Desktop version

Before change:
1. In Android PaymentHandlers, Payment App could circumvent the security
check by delaying onload.
2. toolbar url get updated on navigation end.

After change:
1. Payment Apps are no longer able to circumvent the check this way.
2. toolbar url get updated on navigation start.

Change:
1.
* Add ssl check on security state changed
* Move ssl check from page load end to navigation end
2. Move updating url from navigation end to navigation start.
3. Prevent PaymentHandlerCoordinator from calling hider in hider.
4. Prevent WebContents from being used in PaymentHandlerView after
WebContents is destroyed.

Bug: 1035903

Change-Id: I57821d7e6fbbdc10da2387aff7f8962ae4338716
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037017
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738610}
parent 36290334
......@@ -25,6 +25,7 @@ import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.components.payments.MethodStrings;
import org.chromium.components.payments.PaymentHandlerHost;
import org.chromium.content_public.browser.NavigationHandle;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.browser.WebContents;
import org.chromium.payments.mojom.PaymentAddress;
......@@ -424,7 +425,16 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface
public static void addTabObserverForPaymentRequestTab(Tab tab) {
tab.addObserver(new EmptyTabObserver() {
@Override
public void onPageLoadFinished(Tab tab, String url) {
public void onDidFinishNavigation(Tab tab, NavigationHandle navigationHandle){} {
// Notify closing payment app window so as to abort payment if unsecure.
WebContents webContents = tab.getWebContents();
if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(webContents)) {
onClosingPaymentAppWindowForInsecureNavigation(webContents);
}
}
@Override
public void onSSLStateUpdated(Tab tab) {
// Notify closing payment app window so as to abort payment if unsecure.
WebContents webContents = tab.getWebContents();
if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(webContents)) {
......
......@@ -117,6 +117,7 @@ public class PaymentHandlerCoordinator {
public void hide() {
if (mHider == null) return;
mHider.run();
mHider = null;
}
/**
......
......@@ -15,6 +15,7 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
import org.chromium.content_public.browser.NavigationHandle;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.WebContentsObserver;
import org.chromium.ui.modelutil.PropertyModel;
......@@ -57,14 +58,6 @@ import org.chromium.ui.modelutil.PropertyModel;
mPaymentHandlerUiObserver = observer;
}
private void closeUIForInsecureNavigation() {
mHandler.post(() -> {
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindowForInsecureNavigation(
mWebContentsRef);
mHider.run();
});
}
// BottomSheetObserver:
@Override
public void onSheetStateChanged(@SheetState int newState) {
......@@ -100,7 +93,17 @@ import org.chromium.ui.modelutil.PropertyModel;
// WebContentsObserver:
@Override
public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
public void didFinishNavigation(NavigationHandle navigationHandle) {
if (navigationHandle.isSameDocument()) return;
closeIfInsecure();
}
@Override
public void didChangeVisibleSecurityState() {
closeIfInsecure();
}
private void closeIfInsecure() {
if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(mWebContentsRef)) {
closeUIForInsecureNavigation();
}
......@@ -111,6 +114,14 @@ import org.chromium.ui.modelutil.PropertyModel;
closeUIForInsecureNavigation();
}
private void closeUIForInsecureNavigation() {
mHandler.post(() -> {
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindowForInsecureNavigation(
mWebContentsRef);
mHider.run();
});
}
@Override
public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {
mHandler.post(() -> {
......
......@@ -81,7 +81,7 @@ import org.chromium.ui.base.ActivityWindowAndroid;
/* Resize ThinWebView to reflow the web-contents. */
private void reflowWebContents(float heightFraction) {
// Scale mThinWebView to make the web-content fit into the visible area of the bottom-sheet.
if (mThinWebView.getView() == null) return;
if (mThinWebView.getView() == null || mWebContents.isDestroyed()) return;
LayoutParams params = (LayoutParams) mThinWebView.getView().getLayoutParams();
params.height = Math.max(0, (int) (mTabHeight * heightFraction) - mToolbarHeightPx);
mThinWebView.getView().setLayoutParams(params);
......
......@@ -79,7 +79,11 @@ import java.net.URISyntaxException;
public void didFinishNavigation(NavigationHandle navigation) {
if (!navigation.hasCommitted() || !navigation.isInMainFrame()) return;
mModel.set(PaymentHandlerToolbarProperties.PROGRESS_VISIBLE, false);
}
@Override
public void didStartNavigation(NavigationHandle navigation) {
if (!navigation.isInMainFrame() || navigation.isSameDocument()) return;
String url = navigation.getUrl();
try {
mModel.set(PaymentHandlerToolbarProperties.URL, new URI(url));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment