Commit 93c3f320 authored by Kurt Horimoto's avatar Kurt Horimoto Committed by Commit Bot

[iOS] Use non-modal alerts for app launcher navigations.

When the kNonModalDialogs feature is enabled, app launch alerts will be
presented using OverlayPresenter in the kWebContentArea modlity.
This prevents a page from DOS'ing the entire app by repeated app
launch navigation requests since the user can close the tab.

Bug: 976919, 989296, 989316
Change-Id: Id2044d693766dccaf5c6cfa3df679b5ee2581722
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726667
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Auto-Submit: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684051}
parent 56939f88
...@@ -21,8 +21,11 @@ source_set("app_launcher") { ...@@ -21,8 +21,11 @@ source_set("app_launcher") {
"//components/reading_list/core:core", "//components/reading_list/core:core",
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/overlays",
"//ios/chrome/browser/overlays/public/web_content_area",
"//ios/chrome/browser/reading_list", "//ios/chrome/browser/reading_list",
"//ios/chrome/browser/u2f", "//ios/chrome/browser/u2f",
"//ios/chrome/browser/ui/dialogs:feature_flags",
"//ios/web/common", "//ios/web/common",
"//ios/web/public", "//ios/web/public",
"//url", "//url",
......
...@@ -16,6 +16,8 @@ typedef NS_ENUM(NSInteger, ExternalAppLaunchPolicy) { ...@@ -16,6 +16,8 @@ typedef NS_ENUM(NSInteger, ExternalAppLaunchPolicy) {
// application or launch it. // application or launch it.
ExternalAppLaunchPolicyPrompt, ExternalAppLaunchPolicyPrompt,
// Block launching the application for this session. // Block launching the application for this session.
// TODO(crbug.com/989316): Remove this policy once non-modal dialogs are used
// by default.
ExternalAppLaunchPolicyBlock, ExternalAppLaunchPolicyBlock,
}; };
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#import "ios/chrome/browser/chrome_url_util.h" #import "ios/chrome/browser/chrome_url_util.h"
#include "ios/chrome/browser/reading_list/reading_list_model_factory.h" #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
#import "ios/chrome/browser/u2f/u2f_tab_helper.h" #import "ios/chrome/browser/u2f/u2f_tab_helper.h"
#import "ios/chrome/browser/ui/dialogs/dialog_features.h"
#import "ios/web/common/url_scheme_util.h" #import "ios/web/common/url_scheme_util.h"
#import "ios/web/public/navigation/navigation_item.h" #import "ios/web/public/navigation/navigation_item.h"
#import "ios/web/public/navigation/navigation_manager.h" #import "ios/web/public/navigation/navigation_manager.h"
...@@ -148,11 +149,14 @@ bool AppLauncherTabHelper::RequestToLaunchApp(const GURL& url, ...@@ -148,11 +149,14 @@ bool AppLauncherTabHelper::RequestToLaunchApp(const GURL& url,
launchAppWithURL:copied_url launchAppWithURL:copied_url
linkTransition:YES]; linkTransition:YES];
} else { } else {
// TODO(crbug.com/674649): Once non modal dialogs are implemented, if (!base::FeatureList::IsEnabled(dialogs::kNonModalDialogs)) {
// update this to always prompt instead of blocking the app. // Only block app launches if the app launch alert is being
// displayed modally since DOS attacks are not possible when the
// app launch alert is presented non-modally.
[abuse_detector_ blockLaunchingAppURL:copied_url [abuse_detector_ blockLaunchingAppURL:copied_url
fromSourcePageURL:copied_source_page_url]; fromSourcePageURL:copied_source_page_url];
} }
}
is_prompt_active_ = false; is_prompt_active_ = false;
}]; }];
return true; return true;
......
...@@ -19,6 +19,9 @@ source_set("app_launcher") { ...@@ -19,6 +19,9 @@ source_set("app_launcher") {
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/app_launcher", "//ios/chrome/browser/app_launcher",
"//ios/chrome/browser/app_launcher:feature_flags", "//ios/chrome/browser/app_launcher:feature_flags",
"//ios/chrome/browser/overlays",
"//ios/chrome/browser/overlays/public/web_content_area",
"//ios/chrome/browser/ui/dialogs:feature_flags",
"//ios/public/provider/chrome/browser", "//ios/public/provider/chrome/browser",
"//ios/public/provider/chrome/browser/mailto", "//ios/public/provider/chrome/browser/mailto",
"//net", "//net",
...@@ -38,6 +41,7 @@ source_set("unit_tests") { ...@@ -38,6 +41,7 @@ source_set("unit_tests") {
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//ios/chrome/app/strings:ios_strings_grit", "//ios/chrome/app/strings:ios_strings_grit",
"//ios/chrome/browser/app_launcher",
"//ios/chrome/browser/app_launcher:feature_flags", "//ios/chrome/browser/app_launcher:feature_flags",
"//ios/chrome/test:test_support", "//ios/chrome/test:test_support",
"//ios/web/public/test/fakes", "//ios/web/public/test/fakes",
......
...@@ -6,12 +6,19 @@ ...@@ -6,12 +6,19 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#include "base/bind.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h" #include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#import "ios/chrome/browser/app_launcher/app_launcher_tab_helper.h"
#import "ios/chrome/browser/overlays/public/overlay_request.h"
#import "ios/chrome/browser/overlays/public/overlay_request_queue.h"
#import "ios/chrome/browser/overlays/public/overlay_response.h"
#import "ios/chrome/browser/overlays/public/web_content_area/app_launcher_alert_overlay.h"
#include "ios/chrome/browser/procedural_block_types.h" #include "ios/chrome/browser/procedural_block_types.h"
#import "ios/chrome/browser/ui/app_launcher/app_launcher_util.h" #import "ios/chrome/browser/ui/app_launcher/app_launcher_util.h"
#import "ios/chrome/browser/ui/dialogs/dialog_features.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h" #include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#include "ios/public/provider/chrome/browser/mailto/mailto_handler_provider.h" #include "ios/public/provider/chrome/browser/mailto/mailto_handler_provider.h"
...@@ -33,6 +40,21 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) { ...@@ -33,6 +40,21 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) {
UMA_HISTOGRAM_BOOLEAN("Tab.ExternalApplicationOpened", user_accepted); UMA_HISTOGRAM_BOOLEAN("Tab.ExternalApplicationOpened", user_accepted);
} }
// Callback for the app launcher alert overlay.
void AppLauncherOverlayCallback(ProceduralBlockWithBool app_launch_completion,
OverlayResponse* response) {
DCHECK(app_launch_completion);
if (!response) {
app_launch_completion(NO);
return;
}
AppLauncherAlertOverlayResponseInfo* info =
response->GetInfo<AppLauncherAlertOverlayResponseInfo>();
bool allow_navigation = info ? info->allow_navigation() : false;
app_launch_completion(allow_navigation);
}
} // namespace } // namespace
@interface AppLauncherCoordinator () @interface AppLauncherCoordinator ()
...@@ -60,6 +82,7 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) { ...@@ -60,6 +82,7 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) {
acceptActionTitle:(NSString*)acceptActionTitle acceptActionTitle:(NSString*)acceptActionTitle
rejectActionTitle:(NSString*)rejectActionTitle rejectActionTitle:(NSString*)rejectActionTitle
completionHandler:(ProceduralBlockWithBool)completionHandler { completionHandler:(ProceduralBlockWithBool)completionHandler {
DCHECK(!base::FeatureList::IsEnabled(dialogs::kNonModalDialogs));
UIAlertController* alertController = UIAlertController* alertController =
[UIAlertController alertControllerWithTitle:nil [UIAlertController alertControllerWithTitle:nil
message:message message:message
...@@ -86,23 +109,37 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) { ...@@ -86,23 +109,37 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) {
// Shows an alert that the app will open in another application. If the user // Shows an alert that the app will open in another application. If the user
// accepts, the |URL| is launched. // accepts, the |URL| is launched.
- (void)showAlertAndLaunchAppURL:(const GURL&)URL { - (void)showAlertAndLaunchAppURL:(const GURL&)URL
webState:(web::WebState*)webState {
NSString* prompt = l10n_util::GetNSString(IDS_IOS_OPEN_IN_ANOTHER_APP); NSString* prompt = l10n_util::GetNSString(IDS_IOS_OPEN_IN_ANOTHER_APP);
NSString* openLabel = NSString* openLabel =
l10n_util::GetNSString(IDS_IOS_APP_LAUNCHER_OPEN_APP_BUTTON_LABEL); l10n_util::GetNSString(IDS_IOS_APP_LAUNCHER_OPEN_APP_BUTTON_LABEL);
NSString* cancelLabel = l10n_util::GetNSString(IDS_CANCEL); NSString* cancelLabel = l10n_util::GetNSString(IDS_CANCEL);
NSURL* copiedURL = net::NSURLWithGURL(URL); NSURL* copiedURL = net::NSURLWithGURL(URL);
[self showAlertWithMessage:prompt ProceduralBlockWithBool completion = ^(BOOL userAccepted) {
acceptActionTitle:openLabel
rejectActionTitle:cancelLabel
completionHandler:^(BOOL userAccepted) {
RecordUserAcceptedAppLaunchMetric(userAccepted); RecordUserAcceptedAppLaunchMetric(userAccepted);
if (userAccepted) { if (userAccepted) {
[[UIApplication sharedApplication] openURL:copiedURL [[UIApplication sharedApplication] openURL:copiedURL
options:@{} options:@{}
completionHandler:nil]; completionHandler:nil];
} }
}]; };
if (base::FeatureList::IsEnabled(dialogs::kNonModalDialogs)) {
std::unique_ptr<OverlayRequest> request =
OverlayRequest::CreateWithConfig<AppLauncherAlertOverlayRequestConfig>(
/* is_repeated_request= */ false);
request->set_callback(
base::BindOnce(&AppLauncherOverlayCallback, completion));
OverlayRequestQueue::FromWebState(webState,
OverlayModality::kWebContentArea)
->AddRequest(std::move(request));
} else {
[self showAlertWithMessage:prompt
acceptActionTitle:openLabel
rejectActionTitle:cancelLabel
completionHandler:completion];
}
} }
#pragma mark - AppLauncherTabHelperDelegate #pragma mark - AppLauncherTabHelperDelegate
...@@ -115,8 +152,9 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) { ...@@ -115,8 +152,9 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) {
UIApplicationStateActive) { UIApplicationStateActive) {
return NO; return NO;
} }
web::WebState* webState = tabHelper->web_state();
if (UrlHasAppStoreScheme(URL)) { if (UrlHasAppStoreScheme(URL)) {
[self showAlertAndLaunchAppURL:URL]; [self showAlertAndLaunchAppURL:URL webState:webState];
return YES; return YES;
} }
...@@ -136,7 +174,7 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) { ...@@ -136,7 +174,7 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) {
options:@{} options:@{}
completionHandler:nil]; completionHandler:nil];
} else { } else {
[self showAlertAndLaunchAppURL:URL]; [self showAlertAndLaunchAppURL:URL webState:webState];
} }
return YES; return YES;
} }
...@@ -161,14 +199,26 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) { ...@@ -161,14 +199,26 @@ void RecordUserAcceptedAppLaunchMetric(BOOL user_accepted) {
l10n_util::GetNSString(IDS_IOS_OPEN_REPEATEDLY_ANOTHER_APP_ALLOW); l10n_util::GetNSString(IDS_IOS_OPEN_REPEATEDLY_ANOTHER_APP_ALLOW);
NSString* blockLaunchTitle = NSString* blockLaunchTitle =
l10n_util::GetNSString(IDS_IOS_OPEN_REPEATEDLY_ANOTHER_APP_BLOCK); l10n_util::GetNSString(IDS_IOS_OPEN_REPEATEDLY_ANOTHER_APP_BLOCK);
ProceduralBlockWithBool completion = ^(BOOL userAllowed) {
UMA_HISTOGRAM_BOOLEAN("IOS.RepeatedExternalAppPromptResponse", userAllowed);
completionHandler(userAllowed);
};
if (base::FeatureList::IsEnabled(dialogs::kNonModalDialogs)) {
std::unique_ptr<OverlayRequest> request =
OverlayRequest::CreateWithConfig<AppLauncherAlertOverlayRequestConfig>(
/* is_repeated_request= */ true);
request->set_callback(
base::BindOnce(&AppLauncherOverlayCallback, completion));
OverlayRequestQueue::FromWebState(tabHelper->web_state(),
OverlayModality::kWebContentArea)
->AddRequest(std::move(request));
} else {
[self showAlertWithMessage:message [self showAlertWithMessage:message
acceptActionTitle:allowLaunchTitle acceptActionTitle:allowLaunchTitle
rejectActionTitle:blockLaunchTitle rejectActionTitle:blockLaunchTitle
completionHandler:^(BOOL userAllowed) { completionHandler:completion];
UMA_HISTOGRAM_BOOLEAN("IOS.RepeatedExternalAppPromptResponse", }
userAllowed);
completionHandler(userAllowed);
}];
} }
@end @end
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h" #include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#import "ios/chrome/browser/app_launcher/app_launcher_tab_helper.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/scoped_key_window.h" #import "ios/chrome/test/scoped_key_window.h"
#import "ios/web/public/test/fakes/test_web_state.h" #import "ios/web/public/test/fakes/test_web_state.h"
...@@ -33,10 +34,15 @@ class AppLauncherCoordinatorTest : public PlatformTest { ...@@ -33,10 +34,15 @@ class AppLauncherCoordinatorTest : public PlatformTest {
initWithBaseViewController:base_view_controller_]; initWithBaseViewController:base_view_controller_];
application_ = OCMClassMock([UIApplication class]); application_ = OCMClassMock([UIApplication class]);
OCMStub([application_ sharedApplication]).andReturn(application_); OCMStub([application_ sharedApplication]).andReturn(application_);
AppLauncherTabHelper::CreateForWebState(&web_state_, nil, nil);
} }
~AppLauncherCoordinatorTest() override { [application_ stopMocking]; } ~AppLauncherCoordinatorTest() override { [application_ stopMocking]; }
AppLauncherTabHelper* tab_helper() {
return AppLauncherTabHelper::FromWebState(&web_state_);
}
web::TestWebState web_state_;
UIViewController* base_view_controller_ = nil; UIViewController* base_view_controller_ = nil;
ScopedKeyWindow scoped_key_window_; ScopedKeyWindow scoped_key_window_;
AppLauncherCoordinator* coordinator_ = nil; AppLauncherCoordinator* coordinator_ = nil;
...@@ -46,7 +52,7 @@ class AppLauncherCoordinatorTest : public PlatformTest { ...@@ -46,7 +52,7 @@ class AppLauncherCoordinatorTest : public PlatformTest {
// Tests that an itunes URL shows an alert. // Tests that an itunes URL shows an alert.
TEST_F(AppLauncherCoordinatorTest, ItmsUrlShowsAlert) { TEST_F(AppLauncherCoordinatorTest, ItmsUrlShowsAlert) {
BOOL app_exists = [coordinator_ appLauncherTabHelper:nullptr BOOL app_exists = [coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("itms://1234") launchAppWithURL:GURL("itms://1234")
linkTransition:NO]; linkTransition:NO];
EXPECT_TRUE(app_exists); EXPECT_TRUE(app_exists);
...@@ -68,7 +74,7 @@ TEST_F(AppLauncherCoordinatorTest, AppUrlLaunchesApp) { ...@@ -68,7 +74,7 @@ TEST_F(AppLauncherCoordinatorTest, AppUrlLaunchesApp) {
#pragma clang diagnostic ignored "-Wdeprecated-declarations" #pragma clang diagnostic ignored "-Wdeprecated-declarations"
OCMExpect([application_ openURL:[NSURL URLWithString:@"some-app://1234"]]); OCMExpect([application_ openURL:[NSURL URLWithString:@"some-app://1234"]]);
#pragma clang diagnostic pop #pragma clang diagnostic pop
[coordinator_ appLauncherTabHelper:nullptr [coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("some-app://1234") launchAppWithURL:GURL("some-app://1234")
linkTransition:NO]; linkTransition:NO];
[application_ verify]; [application_ verify];
...@@ -83,7 +89,7 @@ TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlLaunchesApp) { ...@@ -83,7 +89,7 @@ TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlLaunchesApp) {
OCMExpect([application_ openURL:[NSURL URLWithString:@"some-app://1234"] OCMExpect([application_ openURL:[NSURL URLWithString:@"some-app://1234"]
options:@{} options:@{}
completionHandler:nil]); completionHandler:nil]);
[coordinator_ appLauncherTabHelper:nullptr [coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("some-app://1234") launchAppWithURL:GURL("some-app://1234")
linkTransition:YES]; linkTransition:YES];
[application_ verify]; [application_ verify];
...@@ -95,7 +101,7 @@ TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlShowsPrompt) { ...@@ -95,7 +101,7 @@ TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlShowsPrompt) {
// Make sure that the new AppLauncherRefresh logic is enabled. // Make sure that the new AppLauncherRefresh logic is enabled.
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kAppLauncherRefresh); scoped_feature_list.InitAndEnableFeature(kAppLauncherRefresh);
[coordinator_ appLauncherTabHelper:nullptr [coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("some-app://1234") launchAppWithURL:GURL("some-app://1234")
linkTransition:NO]; linkTransition:NO];
ASSERT_TRUE([base_view_controller_.presentedViewController ASSERT_TRUE([base_view_controller_.presentedViewController
...@@ -121,7 +127,7 @@ TEST_F(AppLauncherCoordinatorTest, NoApplicationForUrl) { ...@@ -121,7 +127,7 @@ TEST_F(AppLauncherCoordinatorTest, NoApplicationForUrl) {
.andReturn(NO); .andReturn(NO);
#pragma clang diagnostic pop #pragma clang diagnostic pop
BOOL app_exists = BOOL app_exists =
[coordinator_ appLauncherTabHelper:nullptr [coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("no-app-installed://1234") launchAppWithURL:GURL("no-app-installed://1234")
linkTransition:NO]; linkTransition:NO];
EXPECT_FALSE(app_exists); EXPECT_FALSE(app_exists);
......
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