Commit 4eb2eae2 authored by Henrique Grandinetti's avatar Henrique Grandinetti Committed by Commit Bot

Require Parent Access Code to change the clock on child session

This CL adds a method to the login data dispatcher that shows the
parent access widget and uses it to only allow parents to change the
clock when there is a child signed in or a child is a device owner.

Bug: 921145
Change-Id: Ifd83760f8b9000576570f85c895d34c76c4c8bb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1621392Reviewed-by: default avatarAga Wronska <agawronska@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Henrique Grandinetti <hgrandinetti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665149}
parent 039da5b7
......@@ -9,6 +9,7 @@
#include "ash/focus_cycler.h"
#include "ash/login/ui/lock_screen.h"
#include "ash/login/ui/login_data_dispatcher.h"
#include "ash/login/ui/parent_access_widget.h"
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/root_window_controller.h"
#include "ash/session/session_controller_impl.h"
......@@ -180,7 +181,7 @@ void LoginScreenController::AuthenticateUserWithEasyUnlock(
}
void LoginScreenController::ValidateParentAccessCode(
const base::Optional<AccountId>& account_id,
const AccountId& account_id,
const std::string& code,
OnParentAccessValidation callback) {
if (!login_screen_client_) {
......@@ -300,6 +301,13 @@ LoginScreenModel* LoginScreenController::GetModel() {
return &login_data_dispatcher_;
}
void LoginScreenController::ShowParentAccessWidget(
const AccountId& child_account_id,
base::RepeatingCallback<void(bool success)> callback) {
parent_access_widget_ =
std::make_unique<ash::ParentAccessWidget>(child_account_id, callback);
}
void LoginScreenController::SetClient(mojom::LoginScreenClientPtr client) {
login_screen_client_ = std::move(client);
}
......
......@@ -21,6 +21,7 @@ class PrefRegistrySimple;
namespace ash {
class ParentAccessWidget;
class SystemTrayNotifier;
// LoginScreenController implements mojom::LoginScreen and wraps the
......@@ -74,7 +75,7 @@ class ASH_EXPORT LoginScreenController : public mojom::LoginScreen,
OnAuthenticateCallback callback);
void EnrollUserWithExternalBinary(OnAuthenticateCallback callback);
void AuthenticateUserWithEasyUnlock(const AccountId& account_id);
void ValidateParentAccessCode(const base::Optional<AccountId>& account_id,
void ValidateParentAccessCode(const AccountId& account_id,
const std::string& code,
OnParentAccessValidation callback);
void HardlockPod(const AccountId& account_id);
......@@ -109,6 +110,9 @@ class ASH_EXPORT LoginScreenController : public mojom::LoginScreen,
// LoginScreen:
LoginScreenModel* GetModel() override;
void ShowParentAccessWidget(
const AccountId& child_account_id,
base::RepeatingCallback<void(bool success)> callback) override;
// mojom::LoginScreen:
void SetClient(mojom::LoginScreenClientPtr client) override;
......@@ -177,6 +181,8 @@ class ASH_EXPORT LoginScreenController : public mojom::LoginScreen,
// If set to false, all auth requests will forcibly fail.
ForceFailAuth force_fail_auth_for_debug_overlay_ = ForceFailAuth::kOff;
std::unique_ptr<ParentAccessWidget> parent_access_widget_;
base::WeakPtrFactory<LoginScreenController> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(LoginScreenController);
......
......@@ -60,7 +60,7 @@ void MockLoginScreenClient::EnrollUserWithExternalBinary(
}
void MockLoginScreenClient::ValidateParentAccessCode(
const base::Optional<AccountId>& account_id,
const AccountId& account_id,
const std::string& code,
ValidateParentAccessCodeCallback callback) {
ValidateParentAccessCode_(account_id, code, callback);
......
......@@ -30,7 +30,7 @@ class MockLoginScreenClient : public mojom::LoginScreenClient {
MOCK_METHOD1(EnrollUserWithExternalBinary_,
void(EnrollUserWithExternalBinaryCallback& callback));
MOCK_METHOD3(ValidateParentAccessCode_,
void(const base::Optional<AccountId>& account_id,
void(const AccountId& account_id,
const std::string& access_code,
ValidateParentAccessCodeCallback& callback));
......@@ -74,7 +74,7 @@ class MockLoginScreenClient : public mojom::LoginScreenClient {
void EnrollUserWithExternalBinary(
EnrollUserWithExternalBinaryCallback callback) override;
void ValidateParentAccessCode(
const base::Optional<AccountId>& account_id,
const AccountId& account_id,
const std::string& code,
ValidateParentAccessCodeCallback callback) override;
MOCK_METHOD1(AuthenticateUserWithEasyUnlock,
......
......@@ -391,7 +391,7 @@ ParentAccessView::Callbacks::Callbacks(const Callbacks& other) = default;
ParentAccessView::Callbacks::~Callbacks() = default;
ParentAccessView::ParentAccessView(const base::Optional<AccountId>& account_id,
ParentAccessView::ParentAccessView(const AccountId& account_id,
const Callbacks& callbacks)
: NonAccessibleView(kParentAccessViewClassName),
callbacks_(callbacks),
......
......@@ -80,8 +80,7 @@ class ASH_EXPORT ParentAccessView : public NonAccessibleView,
// specific child, when |account_id| is set, or to any child signed in the
// device, when it is empty. |callbacks| will be called when user performs
// certain actions.
ParentAccessView(const base::Optional<AccountId>& account_id,
const Callbacks& callbacks);
ParentAccessView(const AccountId& account_id, const Callbacks& callbacks);
~ParentAccessView() override;
// views::View:
......@@ -123,7 +122,7 @@ class ASH_EXPORT ParentAccessView : public NonAccessibleView,
// Account id of the user that parent access code is processed for. When
// empty, the code is processed for all the children signed in the device.
const base::Optional<AccountId> account_id_;
const AccountId account_id_;
State state_ = State::kNormal;
......
......@@ -69,7 +69,7 @@ class ParentAccessViewTest : public LoginTestBase {
access_granted ? ++successful_validation_ : ++back_action_;
}
const base::Optional<AccountId> account_id_;
const AccountId account_id_;
std::unique_ptr<MockLoginScreenClient> login_client_;
// Number of times the view was dismissed with back button.
......
......@@ -15,9 +15,9 @@
namespace ash {
ParentAccessWidget::ParentAccessWidget(
const base::Optional<AccountId>& account_id,
const OnExitCallback& callback) {
ParentAccessWidget::ParentAccessWidget(const AccountId& account_id,
const OnExitCallback& callback)
: callback_(callback) {
views::Widget::InitParams widget_params;
// Using window frameless to be able to get focus on the view input fields,
// which does not work with popup type.
......@@ -40,7 +40,8 @@ ParentAccessWidget::ParentAccessWidget(
widget_->Init(widget_params);
ParentAccessView::Callbacks callbacks;
callbacks.on_finished = callback;
callbacks.on_finished = base::BindRepeating(&ParentAccessWidget::OnExit,
weak_factory_.GetWeakPtr());
widget_->SetContentsView(new ParentAccessView(account_id, callbacks));
widget_->CenterWindow(widget_->GetContentsView()->GetPreferredSize());
......@@ -50,4 +51,9 @@ ParentAccessWidget::ParentAccessWidget(
ParentAccessWidget::~ParentAccessWidget() = default;
void ParentAccessWidget::OnExit(bool success) {
callback_.Run(success);
widget_->Close();
}
} // namespace ash
......@@ -7,8 +7,9 @@
#include <memory>
#include "base/callback_forward.h"
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
class AccountId;
......@@ -24,19 +25,26 @@ class ParentAccessWidget {
public:
using OnExitCallback = base::RepeatingCallback<void(bool success)>;
// Creates and shows the widget. When |account_id| is set, the parent
// Creates and shows the widget. When |account_id| is valid, the parent
// access code is validated using the configuration for the provided account,
// when it is empty it tries to validate the access code to any child signed
// in the device. The |callback| is called when (a) the validation is
// successful or (b) the back button is pressed.
ParentAccessWidget(const base::Optional<AccountId>& account_id,
ParentAccessWidget(const AccountId& account_id,
const OnExitCallback& callback);
~ParentAccessWidget();
private:
// Closes the widget and forwards the result to the validation to |callback_|.
void OnExit(bool success);
std::unique_ptr<views::Widget> widget_;
OnExitCallback callback_;
base::WeakPtrFactory<ParentAccessWidget> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(ParentAccessWidget);
};
......
......@@ -8,6 +8,9 @@
#include <string>
#include "ash/public/cpp/ash_public_export.h"
#include "base/callback_forward.h"
class AccountId;
namespace ash {
......@@ -23,6 +26,16 @@ class ASH_PUBLIC_EXPORT LoginScreen {
virtual LoginScreenModel* GetModel() = 0;
// Shows a standalone Parent Access dialog. If |child_account_id| is valid, it
// validates the parent access code for that child only, when it is empty it
// validates the code for any child signed in the device. |callback| is
// invoked when the back button is clicked or the correct code was entered.
// Note: this is intended for children only. If a non child account id is
// provided, the validation will necessarily fail.
virtual void ShowParentAccessWidget(
const AccountId& child_account_id,
base::RepeatingCallback<void(bool success)> callback) = 0;
protected:
LoginScreen();
virtual ~LoginScreen();
......
......@@ -158,8 +158,9 @@ interface LoginScreenClient {
// that is signed in the device. Passes validation result in the callback.
// Note: This should only be used for child user, it will always return false
// when a non-child id is used.
// TODO(crbug.com/965479): move this to a more appropriate place.
ValidateParentAccessCode(
signin.mojom.AccountId? account_id,
signin.mojom.AccountId account_id,
string access_code) => (bool access_code_valid);
// Request to hard lock the user pod.
......
......@@ -12,13 +12,31 @@
#include "base/time/clock.h"
#include "base/time/default_clock.h"
#include "base/timer/timer.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/user_manager/user.h"
#include "components/user_manager/user_manager.h"
namespace chromeos {
namespace parent_access {
namespace {
// Returns true when the device owner is a child.
bool IsDeviceOwnedByChild() {
AccountId owner_account_id =
user_manager::UserManager::Get()->GetOwnerAccountId();
if (owner_account_id.empty())
return false;
const user_manager::User* device_owner =
user_manager::UserManager::Get()->FindUser(owner_account_id);
CHECK(device_owner);
return device_owner->IsChild();
}
} // namespace
// static
void ParentAccessService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterDictionaryPref(prefs::kParentAccessCodeConfig);
......@@ -30,17 +48,31 @@ ParentAccessService& ParentAccessService::Get() {
return *instance;
}
// static
bool ParentAccessService::IsApprovalRequired(SupervisedAction action) {
switch (action) {
case SupervisedAction::kUpdateClock:
if (!base::FeatureList::IsEnabled(
features::kParentAccessCodeForTimeChange)) {
return false;
}
if (user_manager::UserManager::Get()->IsUserLoggedIn())
return user_manager::UserManager::Get()->GetActiveUser()->IsChild();
return IsDeviceOwnedByChild();
}
}
ParentAccessService::ParentAccessService()
: clock_(base::DefaultClock::GetInstance()) {}
ParentAccessService::~ParentAccessService() = default;
bool ParentAccessService::ValidateParentAccessCode(
base::Optional<AccountId> account_id,
const AccountId& account_id,
const std::string& access_code) {
bool validation_result = false;
for (const auto& map_entry : config_source_.config_map()) {
if (!account_id || account_id == map_entry.first) {
if (!account_id.is_valid() || account_id == map_entry.first) {
for (const auto& validator : map_entry.second) {
if (validator->Validate(access_code, clock_->Now())) {
validation_result = true;
......
......@@ -39,16 +39,27 @@ class ParentAccessService {
base::Optional<AccountId> account_id) = 0;
};
// Actions that might require parental approval.
enum class SupervisedAction {
// When Chrome is unable to automatically verify if the OS time is correct
// the user becomes able to manually change the clock. The entry points are
// the settings page (in-session) and the tray bubble (out-session).
kUpdateClock
};
// Registers preferences.
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
// Gets the service singleton.
static ParentAccessService& Get();
// Checks if the provided |action| requires parental approval to be performed.
static bool IsApprovalRequired(SupervisedAction action);
// Checks if |access_code| is valid for the user identified by |account_id|.
// When account_id is empty, this method checks if the |access_code| is valid
// for any child that was added to this device.
bool ValidateParentAccessCode(base::Optional<AccountId> account_id,
bool ValidateParentAccessCode(const AccountId& account_id,
const std::string& access_code);
// Reloads config for the provided user.
......
......@@ -294,7 +294,7 @@ IN_PROC_BROWSER_TEST_F(ParentAccessServiceTest, NoAccountId) {
test_clock_.SetNow(test_value->first);
EXPECT_TRUE(ParentAccessService::Get().ValidateParentAccessCode(
base::nullopt, test_value->second));
EmptyAccountId(), test_value->second));
}
IN_PROC_BROWSER_TEST_F(ParentAccessServiceTest, InvalidAccountId) {
......
......@@ -17,6 +17,9 @@ cr.define('settime', function() {
/** Closes the dialog. */
dialogClose() {}
/** Notifies C++ code that done button was clicked */
doneClicked() {}
}
/** @implements {settime.SetTimeBrowserProxy} */
......@@ -40,6 +43,11 @@ cr.define('settime', function() {
dialogClose() {
chrome.send('dialogClose');
}
/** @override */
doneClicked() {
chrome.send('doneClicked');
}
}
cr.addSingletonGetter(SetTimeBrowserProxyImpl);
......
......@@ -139,7 +139,8 @@
</div>
<div slot="button-container">
<paper-button class="action-button" on-click="onDoneClick_">
<paper-button id="doneButton" class="action-button"
on-click="onDoneClick_">
$i18n{doneButton}
</paper-button>
</div>
......
......@@ -158,6 +158,7 @@ Polymer({
this.addWebUIListener('system-clock-updated', this.updateTime_.bind(this));
this.addWebUIListener(
'system-timezone-changed', this.setTimezone_.bind(this));
this.addWebUIListener('validation-complete', this.saveAndClose_.bind(this));
this.browserProxy_.sendPageReady();
......@@ -227,7 +228,6 @@ Polymer({
if (e.target.validity.valid && e.target.value) {
// Make this the new fallback time in case of future invalid input.
this.prevValues_[e.target.id] = e.target.value;
this.applyTime_();
} else {
// Restore previous value.
e.target.value = this.prevValues_[e.target.id];
......@@ -242,8 +242,22 @@ Polymer({
this.browserProxy_.setTimezone(e.currentTarget.value);
},
/** @private */
/**
* Called when the done button is clicked. Child accounts need parental
* approval to change time, which requires an extra step after the button is
* clicked. This method notifyies the dialog delegate to start the approval
* step, once the approval is granted the 'validation-complete' event is
* triggered invoking saveAndClose_. For regular accounts, this step is
* skipped and saveAndClose_ is called immediately after the button click.
* @private
*/
onDoneClick_: function() {
this.browserProxy_.doneClicked();
},
/** @private */
saveAndClose_: function() {
this.applyTime_();
this.browserProxy_.dialogClose();
},
});
......
......@@ -141,7 +141,7 @@ void LoginScreenClient::AuthenticateUserWithEasyUnlock(
}
void LoginScreenClient::ValidateParentAccessCode(
const base::Optional<AccountId>& account_id,
const AccountId& account_id,
const std::string& access_code,
ValidateParentAccessCodeCallback callback) {
bool result = chromeos::parent_access::ParentAccessService::Get()
......
......@@ -101,7 +101,7 @@ class LoginScreenClient : public ash::mojom::LoginScreenClient {
EnrollUserWithExternalBinaryCallback callback) override;
void AuthenticateUserWithEasyUnlock(const AccountId& account_id) override;
void ValidateParentAccessCode(
const base::Optional<AccountId>& account_id,
const AccountId& account_id,
const std::string& access_code,
ValidateParentAccessCodeCallback callback) override;
void HardlockPod(const AccountId& account_id) override;
......
......@@ -78,6 +78,10 @@ void TestLoginScreen::SetShowParentAccessDialog(bool show) {}
void TestLoginScreen::FocusLoginShelf(bool reverse) {}
void TestLoginScreen::ShowParentAccessWidget(
const AccountId& child_account_id,
base::RepeatingCallback<void(bool success)> callback) {}
ash::LoginScreenModel* TestLoginScreen::GetModel() {
return &test_screen_model_;
}
......
......@@ -52,6 +52,9 @@ class TestLoginScreen : public ash::mojom::LoginScreen,
void SetShowParentAccessButton(bool show) override;
void SetShowParentAccessDialog(bool show) override;
void FocusLoginShelf(bool reverse) override;
void ShowParentAccessWidget(
const AccountId& child_account_id,
base::RepeatingCallback<void(bool success)> callback) override;
// ash::LoginScreen:
ash::LoginScreenModel* GetModel() override;
......
......@@ -8,12 +8,14 @@
#include <memory>
#include "ash/public/cpp/login_screen.h"
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/build_time.h"
#include "base/macros.h"
#include "base/scoped_observer.h"
#include "base/values.h"
#include "chrome/browser/chromeos/child_accounts/parent_access_code/parent_access_service.h"
#include "chrome/browser/chromeos/set_time_dialog.h"
#include "chrome/browser/chromeos/settings/cros_settings.h"
#include "chrome/browser/chromeos/system/timezone_util.h"
......@@ -25,6 +27,7 @@
#include "chromeos/dbus/system_clock/system_clock_client.h"
#include "chromeos/settings/timezone_settings.h"
#include "components/strings/grit/components_strings.h"
#include "components/user_manager/user_manager.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
#include "content/public/browser/web_ui_data_source.h"
......@@ -38,7 +41,7 @@ class SetTimeMessageHandler : public content::WebUIMessageHandler,
public chromeos::SystemClockClient::Observer,
public system::TimezoneSettings::Observer {
public:
SetTimeMessageHandler() = default;
SetTimeMessageHandler() : weak_factory_(this) {}
~SetTimeMessageHandler() override = default;
// WebUIMessageHandler:
......@@ -55,6 +58,9 @@ class SetTimeMessageHandler : public content::WebUIMessageHandler,
"setTimezone",
base::BindRepeating(&SetTimeMessageHandler::OnSetTimezone,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"doneClicked", base::BindRepeating(&SetTimeMessageHandler::DoneClicked,
base::Unretained(this)));
}
void OnJavascriptAllowed() override {
......@@ -113,10 +119,35 @@ class SetTimeMessageHandler : public content::WebUIMessageHandler,
system::SetTimezoneFromUI(profile, timezone_id);
}
void DoneClicked(const base::ListValue* args) {
if (!parent_access::ParentAccessService::IsApprovalRequired(
parent_access::ParentAccessService::SupervisedAction::
kUpdateClock)) {
OnParentAccessValidation(true);
return;
}
AccountId account_id;
if (user_manager::UserManager::Get()->IsUserLoggedIn()) {
account_id =
user_manager::UserManager::Get()->GetActiveUser()->GetAccountId();
}
ash::LoginScreen::Get()->ShowParentAccessWidget(
account_id,
base::BindRepeating(&SetTimeMessageHandler::OnParentAccessValidation,
weak_factory_.GetWeakPtr()));
}
void OnParentAccessValidation(bool success) {
if (success)
FireWebUIListener("validation-complete");
}
ScopedObserver<SystemClockClient, SystemClockClient::Observer>
clock_observer_{this};
ScopedObserver<system::TimezoneSettings, system::TimezoneSettings::Observer>
timezone_observer_{this};
base::WeakPtrFactory<SetTimeMessageHandler> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(SetTimeMessageHandler);
};
......
......@@ -465,6 +465,9 @@ const base::Feature kUseNewAcceptLanguageHeader{
// user device.
const base::Feature kParentAccessCode{"ParentAccessCode",
base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kParentAccessCodeForTimeChange{
"ParentAccessCodeForTimeChange", base::FEATURE_DISABLED_BY_DEFAULT};
#endif
// Delegate permissions to cross-origin iframes when the feature has been
......
......@@ -299,6 +299,9 @@ extern const base::Feature kUseNewAcceptLanguageHeader;
#if defined(OS_CHROMEOS)
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kParentAccessCode;
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kParentAccessCodeForTimeChange;
#endif
COMPONENT_EXPORT(CHROME_FEATURES)
......
......@@ -31,10 +31,8 @@ TEST_F('SetTimeDialogBrowserTest', 'All', function() {
class TestSetTimeBrowserProxy extends TestBrowserProxy {
constructor() {
super([
'sendPageReady',
'setTimeInSeconds',
'setTimezone',
'dialogClose',
'sendPageReady', 'setTimeInSeconds', 'setTimezone', 'dialogClose',
'doneClicked'
]);
}
......@@ -57,6 +55,12 @@ TEST_F('SetTimeDialogBrowserTest', 'All', function() {
dialogClose() {
this.methodCalled('dialogClose');
}
/** @override */
doneClicked() {
this.methodCalled('doneClicked');
cr.webUIListenerCallback('validation-complete');
}
}
suiteSetup(function() {
......@@ -110,7 +114,7 @@ TEST_F('SetTimeDialogBrowserTest', 'All', function() {
const nextWeek = new Date(today.getTime() + 7 * 24 * 60 * 60 * 1000);
dateInput.focus();
dateInput.valueAsDate = nextWeek;
dateInput.blur();
setTimeElement.$$('#doneButton').click();
// Verify the page sends a request to move time forward.
return testBrowserProxy.whenCalled('setTimeInSeconds')
......
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