Skip to content

Commit

Permalink
Projector: Close and disable app if admin disables policy
Browse files Browse the repository at this point in the history
Currently, if the admin disables any of the two policies that control
access to the Projector app, then the SWA remains installed and
enabled. Clicking it results in a 404 error because the WebUIs disable
immediately. If the user has the app already open, then navigating
around hits a 404.

This CL listens for policy changes and immediately closes and disables
the Projector SWA, since we can't uninstall SWAs. The SWA won't
install in the launcher on the next sign in.

Disabled icon in launcher:
https://screenshot.googleplex.com/7VeTCZr3hEvSTyM.png

Error message if click disabled icon:
https://screenshot.googleplex.com/565AxRxCyEba7xE.png

Bug: b/230779397
Change-Id: I2bf12023d5ca27e7ddf59d614695c43471717d42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3632178
Reviewed-by: Jiewei Qian <qjw@chromium.org>
Auto-Submit: Toby Huang <tobyhuang@chromium.org>
Commit-Queue: Li Lin <llin@chromium.org>
Reviewed-by: Li Lin <llin@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001639}
  • Loading branch information
Li Lin authored and Chromium LUCI CQ committed May 10, 2022
1 parent 3abd31b commit 90263e9
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 4 deletions.
1 change: 1 addition & 0 deletions ash/projector/test/mock_projector_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class ASH_EXPORT MockProjectorClient : public ProjectorClient,
MOCK_CONST_METHOD0(IsDriveFsMountFailed, bool());
MOCK_CONST_METHOD0(OpenProjectorApp, void());
MOCK_CONST_METHOD0(MinimizeProjectorApp, void());
MOCK_CONST_METHOD0(CloseProjectorApp, void());
MOCK_CONST_METHOD1(OnNewScreencastPreconditionChanged,
void(const NewScreencastPrecondition&));
MOCK_METHOD1(SetAnnotatorMessageHandler, void(AnnotatorMessageHandler*));
Expand Down
2 changes: 2 additions & 0 deletions ash/public/cpp/projector/projector_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class ASH_PUBLIC_EXPORT ProjectorClient {
virtual void OpenProjectorApp() const = 0;
// Minimizes Projector SWA.
virtual void MinimizeProjectorApp() const = 0;
// Closes Projector SWA.
virtual void CloseProjectorApp() const = 0;

// Registers the AnnotatorMessageHandler that is owned by the WebUI that
// contains the Projector annotator.
Expand Down
58 changes: 58 additions & 0 deletions chrome/browser/ui/ash/projector/projector_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
#include "chrome/browser/ui/ash/projector/projector_client_impl.h"

#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
#include "ash/public/cpp/projector/annotator_tool.h"
#include "ash/public/cpp/projector/projector_controller.h"
#include "ash/public/cpp/projector/projector_new_screencast_precondition.h"
#include "ash/webui/projector_app/annotator_message_handler.h"
#include "ash/webui/projector_app/projector_app_client.h"
#include "ash/webui/projector_app/public/cpp/projector_app_constants.h"
#include "base/bind.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/download/download_prefs.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -19,7 +21,11 @@
#include "chrome/browser/ui/ash/projector/projector_utils.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
#include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h"
#include "chrome/browser/web_applications/system_web_apps/system_web_app_types.h"
#include "chrome/browser/web_applications/web_app_id.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/browser/web_applications/web_app_sync_bridge.h"
#include "chromeos/login/login_state/login_state.h"
#include "components/soda/soda_installer.h"
#include "content/public/browser/download_manager.h"
Expand Down Expand Up @@ -141,6 +147,14 @@ void ProjectorClientImpl::MinimizeProjectorApp() const {
browser->window()->Minimize();
}

void ProjectorClientImpl::CloseProjectorApp() const {
auto* profile = ProfileManager::GetActiveUserProfile();
auto* browser =
FindSystemWebAppBrowser(profile, web_app::SystemAppType::PROJECTOR);
if (browser)
browser->window()->Close();
}

void ProjectorClientImpl::OnNewScreencastPreconditionChanged(
const ash::NewScreencastPrecondition& precondition) const {
ash::ProjectorAppClient* app_client = ash::ProjectorAppClient::Get();
Expand Down Expand Up @@ -217,6 +231,23 @@ void ProjectorClientImpl::OnUserProfileLoaded(const AccountId& account_id) {
MaybeSwitchDriveIntegrationServiceObservation();
}

void ProjectorClientImpl::OnUserSessionStarted(bool is_primary_user) {
if (!is_primary_user || !pref_change_registrar_.IsEmpty())
return;
Profile* profile = ProfileManager::GetActiveUserProfile();
pref_change_registrar_.Init(profile->GetPrefs());
// TOOD(b/232043809): Consider using the disabled system feature policy
// instead.
pref_change_registrar_.Add(
ash::prefs::kProjectorAllowByPolicy,
base::BindRepeating(&ProjectorClientImpl::OnEnablementPolicyChanged,
base::Unretained(this)));
pref_change_registrar_.Add(
ash::prefs::kProjectorDogfoodForFamilyLinkEnabled,
base::BindRepeating(&ProjectorClientImpl::OnEnablementPolicyChanged,
base::Unretained(this)));
}

void ProjectorClientImpl::ActiveUserChanged(user_manager::User* active_user) {
// After user login, the first ActiveUserChanged() might be called before
// profile is loaded.
Expand All @@ -237,3 +268,30 @@ void ProjectorClientImpl::MaybeSwitchDriveIntegrationServiceObservation() {
drive_observation_.Reset();
drive_observation_.Observe(drive_service);
}

void ProjectorClientImpl::OnEnablementPolicyChanged() {
Profile* profile = ProfileManager::GetActiveUserProfile();
web_app::SystemWebAppManager& manager =
web_app::WebAppProvider::GetForSystemWebApps(profile)
->system_web_app_manager();
const bool is_installed =
manager.IsSystemWebApp(ash::kChromeUITrustedProjectorSwaAppId);
// We can't enable or disable the app if it's not already installed.
if (!is_installed)
return;

const bool is_enabled = IsProjectorAppEnabled(profile);
// The policy has changed to disallow the Projector app. Since we can't
// uninstall the Projector SWA until the user signs out and back in, we should
// close and disable the app for this current session.
if (!is_enabled)
CloseProjectorApp();

absl::optional<web_app::AppId> app_id =
GetAppIdForSystemWebApp(profile, web_app::SystemAppType::PROJECTOR);
if (!app_id)
return;

auto* web_app_provider = web_app::WebAppProvider::GetForWebApps(profile);
web_app_provider->sync_bridge().SetAppIsDisabled(app_id.value(), !is_enabled);
}
9 changes: 9 additions & 0 deletions chrome/browser/ui/ash/projector/projector_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/scoped_observation.h"
#include "chrome/browser/ash/drive/drive_integration_service.h"
#include "chrome/browser/speech/speech_recognizer_delegate.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/session_manager/core/session_manager.h"
#include "components/session_manager/core/session_manager_observer.h"
#include "components/soda/constants.h"
Expand Down Expand Up @@ -56,6 +57,7 @@ class ProjectorClientImpl
bool IsDriveFsMountFailed() const override;
void OpenProjectorApp() const override;
void MinimizeProjectorApp() const override;
void CloseProjectorApp() const override;
void OnNewScreencastPreconditionChanged(
const ash::NewScreencastPrecondition& precondition) const override;
void SetAnnotatorMessageHandler(
Expand Down Expand Up @@ -85,6 +87,7 @@ class ProjectorClientImpl

// session_manager::SessionManagerObserver:
void OnUserProfileLoaded(const AccountId& account_id) override;
void OnUserSessionStarted(bool is_primary_user) override;

// user_manager::UserManager::UserSessionStateObserver:
void ActiveUserChanged(user_manager::User* active_user) override;
Expand All @@ -94,6 +97,10 @@ class ProjectorClientImpl
// of active profile when ActiveUserChanged and OnUserProfileLoaded.
void MaybeSwitchDriveIntegrationServiceObservation();

// Called when any of the policies change that control whether the Projector
// app is enabled.
void OnEnablementPolicyChanged();

ash::ProjectorController* const controller_;
ash::AnnotatorMessageHandler* message_handler_;
SpeechRecognizerStatus recognizer_status_ =
Expand All @@ -106,6 +113,8 @@ class ProjectorClientImpl
session_manager::SessionManagerObserver>
session_observation_{this};

PrefChangeRegistrar pref_change_registrar_;

base::ScopedObservation<drive::DriveIntegrationService,
drive::DriveIntegrationServiceObserver>
drive_observation_{this};
Expand Down
175 changes: 175 additions & 0 deletions chrome/browser/ui/ash/projector/projector_client_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <memory>

#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
#include "ash/projector/test/mock_projector_client.h"
#include "ash/public/cpp/projector/projector_client.h"
#include "ash/public/cpp/projector/projector_new_screencast_precondition.h"
Expand All @@ -15,8 +16,13 @@
#include "base/callback_forward.h"
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/apps/app_service/app_icon/app_icon_factory.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/ash/drive/drive_integration_service.h"
#include "chrome/browser/ash/drive/drivefs_test_support.h"
#include "chrome/browser/ash/login/test/fake_gaia_mixin.h"
#include "chrome/browser/ash/login/test/logged_in_user_mixin.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/ash/projector/projector_app_client_impl.h"
#include "chrome/browser/ui/browser.h"
Expand All @@ -25,9 +31,15 @@
#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
#include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h"
#include "chrome/browser/web_applications/system_web_apps/system_web_app_types.h"
#include "chrome/browser/web_applications/web_app_id.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/mixin_based_in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/account_id/account_id.h"
#include "components/services/app_service/public/cpp/app_registry_cache.h"
#include "components/services/app_service/public/cpp/app_types.h"
#include "components/services/app_service/public/cpp/icon_types.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/page_type.h"
Expand All @@ -38,6 +50,25 @@

namespace ash {

namespace {

apps::AppServiceProxy* GetAppServiceProxy(Profile* profile) {
return apps::AppServiceProxyFactory::GetForProfile(profile);
}

// Returns the account id for logging in.
absl::optional<AccountId> GetPrimaryAccountId(bool is_managed) {
if (is_managed) {
return AccountId::FromUserEmailGaiaId(
ash::FakeGaiaMixin::kEnterpriseUser1,
ash::FakeGaiaMixin::kEnterpriseUser1GaiaId);
}
// Use the default FakeGaiaMixin::kFakeUserEmail consumer test account id.
return absl::nullopt;
}

} // namespace

// A class helps to verify enable/disable Drive could invoke
// ProjectorAppClient::Observer::OnDriveFsMountStatusChanged().
class DriveFsMountStatusWaiter : public ProjectorAppClient::Observer {
Expand Down Expand Up @@ -194,6 +225,31 @@ IN_PROC_BROWSER_TEST_F(ProjectorClientTest, MinimizeProjectorApp) {
EXPECT_TRUE(app_browser->window()->IsMinimized());
}

IN_PROC_BROWSER_TEST_F(ProjectorClientTest, CloseProjectorApp) {
auto* profile = browser()->profile();
web_app::WebAppProvider::GetForTest(profile)
->system_web_app_manager()
.InstallSystemAppsForTesting();

client()->OpenProjectorApp();
web_app::FlushSystemWebAppLaunchesForTesting(profile);

// Verify that Projector App is opened.
Browser* app_browser =
FindSystemWebAppBrowser(profile, web_app::SystemAppType::PROJECTOR);
ASSERT_TRUE(app_browser);
content::WebContents* tab =
app_browser->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(tab);
EXPECT_EQ(tab->GetController().GetVisibleEntry()->GetPageType(),
content::PAGE_TYPE_NORMAL);

EXPECT_FALSE(app_browser->IsAttemptingToCloseBrowser());
client()->CloseProjectorApp();
// Verify that Projector App is closing.
EXPECT_TRUE(app_browser->IsAttemptingToCloseBrowser());
}

IN_PROC_BROWSER_TEST_F(ProjectorClientTest, GetDriveFsMountPointPath) {
ASSERT_TRUE(client()->IsDriveFsMounted());
ASSERT_FALSE(client()->IsDriveFsMountFailed());
Expand Down Expand Up @@ -225,4 +281,123 @@ IN_PROC_BROWSER_TEST_F(ProjectorClientTest, DriveUnmountedAndRemounted) {
}
}

// Tests Projector client for child and managed users.
class ProjectorClientManagedTest
: public MixinBasedInProcessBrowserTest,
public testing::WithParamInterface</*IsChild=*/bool> {
protected:
void SetUpOnMainThread() override {
MixinBasedInProcessBrowserTest::SetUpOnMainThread();
logged_in_user_mixin_.LogInUser();
}

bool is_child() const { return GetParam(); }

bool is_managed() const { return !is_child(); }

ProjectorClient* client() { return ProjectorClient::Get(); }

std::string GetPolicy() {
if (is_child())
return ash::prefs::kProjectorDogfoodForFamilyLinkEnabled;
return ash::prefs::kProjectorAllowByPolicy;
}

apps::Readiness GetAppReadiness(const web_app::AppId& app_id) {
apps::Readiness readiness;
bool app_found =
GetAppServiceProxy(browser()->profile())
->AppRegistryCache()
.ForOneApp(app_id, [&readiness](const apps::AppUpdate& update) {
readiness = update.Readiness();
});
EXPECT_TRUE(app_found);
return readiness;
}

absl::optional<apps::IconKey> GetAppIconKey(const web_app::AppId& app_id) {
absl::optional<apps::IconKey> icon_key;
bool app_found =
GetAppServiceProxy(browser()->profile())
->AppRegistryCache()
.ForOneApp(app_id, [&icon_key](const apps::AppUpdate& update) {
icon_key = update.IconKey();
});
EXPECT_TRUE(app_found);
return icon_key;
}

private:
LoggedInUserMixin logged_in_user_mixin_{
&mixin_host_,
is_child() ? LoggedInUserMixin::LogInType::kChild
: LoggedInUserMixin::LogInType::kRegular,
embedded_test_server(),
this,
/*should_launch_browser=*/true,
GetPrimaryAccountId(is_managed())};
};

IN_PROC_BROWSER_TEST_P(ProjectorClientManagedTest,
CantOpenProjectorAppWithoutPolicy) {
auto* profile = browser()->profile();
web_app::WebAppProvider::GetForTest(profile)
->system_web_app_manager()
.InstallSystemAppsForTesting();

client()->OpenProjectorApp();
web_app::FlushSystemWebAppLaunchesForTesting(profile);

// Verify that Projector App is not opened.
Browser* app_browser =
FindSystemWebAppBrowser(profile, web_app::SystemAppType::PROJECTOR);
EXPECT_FALSE(app_browser);
}

// Prevents a regression to b/230779397.
IN_PROC_BROWSER_TEST_P(ProjectorClientManagedTest, DisableThenEnablePolicy) {
auto* profile = browser()->profile();
profile->GetPrefs()->SetBoolean(GetPolicy(), true);
web_app::WebAppProvider::GetForTest(profile)
->system_web_app_manager()
.InstallSystemAppsForTesting();

client()->OpenProjectorApp();
web_app::FlushSystemWebAppLaunchesForTesting(profile);

// Verify the user can open the Projector App when the policy is enabled.
Browser* app_browser =
FindSystemWebAppBrowser(profile, web_app::SystemAppType::PROJECTOR);
ASSERT_TRUE(app_browser);
content::WebContents* tab =
app_browser->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(tab);
EXPECT_EQ(tab->GetController().GetVisibleEntry()->GetPageType(),
content::PAGE_TYPE_NORMAL);

// Suppose the policy flips to false while the user is still signed in and has
// the Projector app open.
profile->GetPrefs()->SetBoolean(GetPolicy(), false);
// The Projector app immediately closes to prevent further access.
EXPECT_TRUE(app_browser->IsAttemptingToCloseBrowser());
// We can't uninstall the Projector SWA until the next session, but the icon
// is greyed out and disabled.
EXPECT_EQ(apps::Readiness::kDisabledByPolicy,
GetAppReadiness(kChromeUITrustedProjectorSwaAppId));
EXPECT_TRUE(apps::IconEffects::kBlocked &
GetAppIconKey(kChromeUITrustedProjectorSwaAppId)->icon_effects);

// The app can re-enable too if it's already installed and the policy flips to
// true.
profile->GetPrefs()->SetBoolean(GetPolicy(), true);
EXPECT_EQ(apps::Readiness::kReady,
GetAppReadiness(kChromeUITrustedProjectorSwaAppId));
EXPECT_FALSE(apps::IconEffects::kBlocked &
GetAppIconKey(kChromeUITrustedProjectorSwaAppId)->icon_effects);
}

INSTANTIATE_TEST_SUITE_P(,
ProjectorClientManagedTest,
/*IsChild=*/testing::Bool());

} // namespace ash
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,11 @@ enum class SystemAppType {
// contact: cros-telemetry@google.com
OS_FEEDBACK = 19,

// Projector (go/projector-player-dd) aims to make it simple for teachers and
// students to record and share instructional videos on a Chromebook. This app
// enables teachers to create a library of custom-tailored instructional
// content that students can search and view at home.
// Projector aka Screencast (go/projector-player-dd) aims to make it simple
// for teachers and students to record and share instructional videos on a
// Chromebook. This app enables teachers to create a library of
// custom-tailored instructional content that students can search and view at
// home.
//
// Source: //ash/webui/projector_app/
// Contact: cros-projector@google.com
Expand Down

0 comments on commit 90263e9

Please sign in to comment.