Commit 902a4b1e authored by Guillaume Jenkins's avatar Guillaume Jenkins Committed by Chromium LUCI CQ

[iOS Enterprise] BrowserSignin: hide promos

Makes the sign-in promo view mediator aware of the kSigninAllowed pref
and makes it skip the sign-in promos in the bookmarks and the settings
if sign-in is disabled by policy.

Also makes a similar change in the sign-in utils so the sign-in promo
after a version upgrade is skipped if sign-in is disabled by policy.

Change-Id: Ia84e43f9db317f9454d662ea998758be66ffa801
Bug: 1155745
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562778
Commit-Queue: Guillaume Jenkins <gujen@google.com>
Reviewed-by: default avatarNohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835793}
parent fc940c2a
...@@ -50,6 +50,7 @@ source_set("authentication") { ...@@ -50,6 +50,7 @@ source_set("authentication") {
"//ios/chrome/browser/ui:feature_flags", "//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/alert_coordinator", "//ios/chrome/browser/ui/alert_coordinator",
"//ios/chrome/browser/ui/authentication/cells", "//ios/chrome/browser/ui/authentication/cells",
"//ios/chrome/browser/ui/authentication/signin:signin_headers",
"//ios/chrome/browser/ui/collection_view/cells", "//ios/chrome/browser/ui/collection_view/cells",
"//ios/chrome/browser/ui/colors", "//ios/chrome/browser/ui/colors",
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
...@@ -107,6 +108,7 @@ source_set("unit_tests") { ...@@ -107,6 +108,7 @@ source_set("unit_tests") {
"//components/consent_auditor", "//components/consent_auditor",
"//components/consent_auditor:test_support", "//components/consent_auditor:test_support",
"//components/pref_registry", "//components/pref_registry",
"//components/prefs",
"//components/signin/public/base", "//components/signin/public/base",
"//components/signin/public/identity_manager", "//components/signin/public/identity_manager",
"//components/sync_preferences", "//components/sync_preferences",
......
...@@ -43,10 +43,12 @@ source_set("signin_impl") { ...@@ -43,10 +43,12 @@ source_set("signin_impl") {
deps = [ deps = [
":signin_headers", ":signin_headers",
":signin_protected", ":signin_protected",
"//components/prefs",
"//components/signin/ios/browser", "//components/signin/ios/browser",
"//components/version_info", "//components/version_info",
"//ios/chrome/app:tests_hook", "//ios/chrome/app:tests_hook",
"//ios/chrome/browser/main:public", "//ios/chrome/browser/main:public",
"//ios/chrome/browser/policy:feature_flags",
"//ios/chrome/browser/signin", "//ios/chrome/browser/signin",
"//ios/chrome/browser/ui/authentication/signin/add_account_signin", "//ios/chrome/browser/ui/authentication/signin/add_account_signin",
"//ios/chrome/browser/ui/authentication/signin/advanced_settings_signin", "//ios/chrome/browser/ui/authentication/signin/advanced_settings_signin",
...@@ -66,9 +68,11 @@ source_set("unit_tests") { ...@@ -66,9 +68,11 @@ source_set("unit_tests") {
deps = [ deps = [
":signin_impl", ":signin_impl",
"//base", "//base",
"//base/test:test_support",
"//components/pref_registry", "//components/pref_registry",
"//components/sync_preferences", "//components/sync_preferences",
"//components/sync_preferences:test_support", "//components/sync_preferences:test_support",
"//ios/chrome/browser:utils",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/main:test_support", "//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/prefs:browser_prefs", "//ios/chrome/browser/prefs:browser_prefs",
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
class ChromeBrowserState; class ChromeBrowserState;
class PrefService;
namespace base { namespace base {
class Version; class Version;
...@@ -26,4 +27,11 @@ void SigninRecordVersionSeen(); ...@@ -26,4 +27,11 @@ void SigninRecordVersionSeen();
// Set the Chromium current version for sign-in. Used for tests only. // Set the Chromium current version for sign-in. Used for tests only.
void SetSigninCurrentVersionForTesting(base::Version* version); void SetSigninCurrentVersionForTesting(base::Version* version);
namespace signin {
// Returns a boolean indicating whether browser sign-in is allowed by policy.
bool IsSigninAllowed(const PrefService* prefs);
} // namespace signin
#endif // IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_SIGNIN_UTILS_H_ #endif // IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_SIGNIN_UTILS_H_
...@@ -6,11 +6,14 @@ ...@@ -6,11 +6,14 @@
#import "base/strings/sys_string_conversions.h" #import "base/strings/sys_string_conversions.h"
#import "base/version.h" #import "base/version.h"
#import "components/prefs/pref_service.h"
#import "components/signin/ios/browser/features.h" #import "components/signin/ios/browser/features.h"
#import "components/signin/public/base/signin_pref_names.h"
#import "components/version_info/version_info.h" #import "components/version_info/version_info.h"
#import "ios/chrome/app/tests_hook.h" #import "ios/chrome/app/tests_hook.h"
#import "ios/chrome/browser/browser_state/chrome_browser_state.h" #import "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/main/browser.h" #import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/policy/policy_features.h"
#import "ios/chrome/browser/signin/authentication_service.h" #import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h" #import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_constants.h" #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_constants.h"
...@@ -61,6 +64,10 @@ bool SigninShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState) { ...@@ -61,6 +64,10 @@ bool SigninShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState) {
if (net::NetworkChangeNotifier::IsOffline()) if (net::NetworkChangeNotifier::IsOffline())
return false; return false;
// Sign-in can be disabled by policy.
if (!signin::IsSigninAllowed(browserState->GetPrefs()))
return false;
AuthenticationService* authService = AuthenticationService* authService =
AuthenticationServiceFactory::GetForBrowserState(browserState); AuthenticationServiceFactory::GetForBrowserState(browserState);
authService->WaitUntilCacheIsPopulated(); authService->WaitUntilCacheIsPopulated();
...@@ -124,3 +131,15 @@ void SigninRecordVersionSeen() { ...@@ -124,3 +131,15 @@ void SigninRecordVersionSeen() {
void SetSigninCurrentVersionForTesting(Version* version) { void SetSigninCurrentVersionForTesting(Version* version) {
g_current_version_for_test = version; g_current_version_for_test = version;
} }
// TODO(crbug.com/1157531): Move the other functions inside the signin namespace
// as well.
namespace signin {
bool IsSigninAllowed(const PrefService* prefs) {
// Sign-in is always allowed if the policy handler isn't installed.
return !ShouldInstallBrowserSigninPolicyHandler() ||
prefs->GetBoolean(prefs::kSigninAllowed);
}
} // namespace signin
...@@ -9,11 +9,15 @@ ...@@ -9,11 +9,15 @@
#include <memory> #include <memory>
#import "base/bind.h" #import "base/bind.h"
#import "base/command_line.h"
#import "base/test/scoped_command_line.h"
#import "base/version.h" #import "base/version.h"
#import "components/pref_registry/pref_registry_syncable.h" #import "components/pref_registry/pref_registry_syncable.h"
#import "components/signin/public/base/signin_pref_names.h"
#import "components/sync_preferences/pref_service_mock_factory.h" #import "components/sync_preferences/pref_service_mock_factory.h"
#import "components/sync_preferences/pref_service_syncable.h" #import "components/sync_preferences/pref_service_syncable.h"
#import "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/chrome_switches.h"
#import "ios/chrome/browser/main/test_browser.h" #import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/prefs/browser_prefs.h" #import "ios/chrome/browser/prefs/browser_prefs.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h" #import "ios/chrome/browser/signin/authentication_service_factory.h"
...@@ -98,7 +102,7 @@ TEST_F(SigninUtilsTest, TestWillNotDisplaySameVersion) { ...@@ -98,7 +102,7 @@ TEST_F(SigninUtilsTest, TestWillNotDisplaySameVersion) {
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice until to major version after. // Should not show the sign-in upgrade twice until two major version after.
TEST_F(SigninUtilsTest, TestWillNotDisplayOneMinorVersion) { TEST_F(SigninUtilsTest, TestWillNotDisplayOneMinorVersion) {
SigninRecordVersionSeen(); SigninRecordVersionSeen();
// Set the future version to be one minor release ahead. // Set the future version to be one minor release ahead.
...@@ -108,7 +112,7 @@ TEST_F(SigninUtilsTest, TestWillNotDisplayOneMinorVersion) { ...@@ -108,7 +112,7 @@ TEST_F(SigninUtilsTest, TestWillNotDisplayOneMinorVersion) {
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice until to major version after. // Should not show the sign-in upgrade twice until two major version after.
TEST_F(SigninUtilsTest, TestWillNotDisplayTwoMinorVersions) { TEST_F(SigninUtilsTest, TestWillNotDisplayTwoMinorVersions) {
SigninRecordVersionSeen(); SigninRecordVersionSeen();
// Set the future version to be two minor releases ahead. // Set the future version to be two minor releases ahead.
...@@ -118,7 +122,7 @@ TEST_F(SigninUtilsTest, TestWillNotDisplayTwoMinorVersions) { ...@@ -118,7 +122,7 @@ TEST_F(SigninUtilsTest, TestWillNotDisplayTwoMinorVersions) {
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice until to major version after. // Should not show the sign-in upgrade twice until two major version after.
TEST_F(SigninUtilsTest, TestWillNotDisplayOneMajorVersion) { TEST_F(SigninUtilsTest, TestWillNotDisplayOneMajorVersion) {
SigninRecordVersionSeen(); SigninRecordVersionSeen();
// Set the future version to be one major release ahead. // Set the future version to be one major release ahead.
...@@ -237,4 +241,33 @@ TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersionBis) { ...@@ -237,4 +241,33 @@ TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersionBis) {
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade if sign-in is disabled by policy.
TEST_F(SigninUtilsTest, TestWillNotShowIfDisabledByPolicy) {
base::test::ScopedCommandLine scoped_command_line;
scoped_command_line.GetProcessCommandLine()->AppendSwitch(
switches::kInstallBrowserSigninHandler);
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->AddIdentities(@[ @"foo1" ]);
chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
}
// signin::IsSigninAllowed should respect the kSigninAllowed pref.
TEST_F(SigninUtilsTest, TestSigninAllowedPref) {
base::test::ScopedCommandLine scoped_command_line;
scoped_command_line.GetProcessCommandLine()->AppendSwitch(
switches::kInstallBrowserSigninHandler);
// Sign-in is allowed by default.
EXPECT_TRUE(signin::IsSigninAllowed(chrome_browser_state_.get()->GetPrefs()));
// When sign-in is disabled by policy, the accessor should return false.
chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
EXPECT_FALSE(
signin::IsSigninAllowed(chrome_browser_state_.get()->GetPrefs()));
}
} // namespace } // namespace
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h" #include "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_consumer.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_consumer.h"
#import "ios/chrome/browser/ui/authentication/signin/signin_utils.h"
#import "ios/chrome/browser/ui/commands/application_commands.h" #import "ios/chrome/browser/ui/commands/application_commands.h"
#import "ios/chrome/browser/ui/commands/show_signin_command.h" #import "ios/chrome/browser/ui/commands/show_signin_command.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
...@@ -384,6 +385,9 @@ const char* AlreadySeenSigninViewPreferenceKey( ...@@ -384,6 +385,9 @@ const char* AlreadySeenSigninViewPreferenceKey(
browserState: browserState:
(ChromeBrowserState*)browserState { (ChromeBrowserState*)browserState {
PrefService* prefs = browserState->GetPrefs(); PrefService* prefs = browserState->GetPrefs();
if (!signin::IsSigninAllowed(prefs)) {
return NO;
}
const char* displayedCountPreferenceKey = const char* displayedCountPreferenceKey =
DisplayedCountPreferenceKey(accessPoint); DisplayedCountPreferenceKey(accessPoint);
if (displayedCountPreferenceKey && if (displayedCountPreferenceKey &&
......
...@@ -7,7 +7,16 @@ ...@@ -7,7 +7,16 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#import "base/test/scoped_command_line.h"
#import "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/base/signin_metrics.h" #include "components/signin/public/base/signin_metrics.h"
#import "components/signin/public/base/signin_pref_names.h"
#import "components/sync_preferences/pref_service_mock_factory.h"
#import "components/sync_preferences/pref_service_syncable.h"
#import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/chrome_switches.h"
#import "ios/chrome/browser/prefs/browser_prefs.h"
#include "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h" #include "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h"
...@@ -17,6 +26,7 @@ ...@@ -17,6 +26,7 @@
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity.h" #import "ios/public/provider/chrome/browser/signin/fake_chrome_identity.h"
#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity_service.h" #import "ios/public/provider/chrome/browser/signin/fake_chrome_identity_service.h"
#import "ios/web/public/test/web_task_environment.h"
#import "testing/platform_test.h" #import "testing/platform_test.h"
#import "third_party/ocmock/OCMock/OCMock.h" #import "third_party/ocmock/OCMock/OCMock.h"
#include "third_party/ocmock/gtest_support.h" #include "third_party/ocmock/gtest_support.h"
...@@ -31,6 +41,10 @@ using base::test::ios::kWaitForUIElementTimeout; ...@@ -31,6 +41,10 @@ using base::test::ios::kWaitForUIElementTimeout;
using base::test::ios::WaitUntilConditionOrTimeout; using base::test::ios::WaitUntilConditionOrTimeout;
using l10n_util::GetNSString; using l10n_util::GetNSString;
using l10n_util::GetNSStringF; using l10n_util::GetNSStringF;
using sync_preferences::PrefServiceMockFactory;
using sync_preferences::PrefServiceSyncable;
using user_prefs::PrefRegistrySyncable;
using web::WebTaskEnvironment;
namespace { namespace {
...@@ -74,6 +88,15 @@ class SigninPromoViewMediatorTest : public PlatformTest { ...@@ -74,6 +88,15 @@ class SigninPromoViewMediatorTest : public PlatformTest {
OCMStub([signin_promo_view_ closeButton]).andReturn(close_button_); OCMStub([signin_promo_view_ closeButton]).andReturn(close_button_);
} }
std::unique_ptr<PrefServiceSyncable> CreatePrefService() {
PrefServiceMockFactory factory;
scoped_refptr<PrefRegistrySyncable> registry(new PrefRegistrySyncable);
std::unique_ptr<PrefServiceSyncable> prefs =
factory.CreateSyncable(registry.get());
RegisterBrowserStatePrefs(registry.get());
return prefs;
}
// Creates the default identity and adds it into the ChromeIdentityService. // Creates the default identity and adds it into the ChromeIdentityService.
void AddDefaultIdentity() { void AddDefaultIdentity() {
expected_default_identity_ = expected_default_identity_ =
...@@ -184,6 +207,9 @@ class SigninPromoViewMediatorTest : public PlatformTest { ...@@ -184,6 +207,9 @@ class SigninPromoViewMediatorTest : public PlatformTest {
CheckSigninWithAccountConfigurator(configurator_); CheckSigninWithAccountConfigurator(configurator_);
} }
// Task environment.
WebTaskEnvironment task_environment_;
// Mediator used for the tests. // Mediator used for the tests.
SigninPromoViewMediator* mediator_; SigninPromoViewMediator* mediator_;
...@@ -369,4 +395,21 @@ TEST_F(SigninPromoViewMediatorTest, ...@@ -369,4 +395,21 @@ TEST_F(SigninPromoViewMediatorTest,
completion(YES); completion(YES);
} }
// Tests that promos aren't shown if browser sign-in is disabled by policy
TEST_F(SigninPromoViewMediatorTest,
ShouldNotDisplaySigninPromoViewIfDisabledByPolicy) {
base::test::ScopedCommandLine scoped_command_line;
scoped_command_line.GetProcessCommandLine()->AppendSwitch(
switches::kInstallBrowserSigninHandler);
CreateMediator(signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS);
TestChromeBrowserState::Builder builder;
builder.SetPrefService(CreatePrefService());
std::unique_ptr<TestChromeBrowserState> browser_state = builder.Build();
browser_state->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
EXPECT_FALSE([SigninPromoViewMediator
shouldDisplaySigninPromoViewWithAccessPoint:signin_metrics::AccessPoint::
ACCESS_POINT_RECENT_TABS
browserState:browser_state.get()]);
}
} // namespace } // namespace
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